From e7f8654fd4983bd0e638b35cb8b2dd21748219da Mon Sep 17 00:00:00 2001 From: Quentin Renard Date: Fri, 4 Oct 2024 15:29:54 +0200 Subject: [PATCH] Making classers.del safer + FilterGraph now stores its FilterContexts to remove them from classers properly --- bit_stream_filter_context.go | 6 ++++-- class_test.go | 18 +++++++++++------- codec_context.go | 6 ++++-- filter_context.go | 7 ++++++- filter_graph.go | 21 ++++++++++++++++++--- format_context.go | 22 ++++++++++++++++------ io_context.go | 12 ++++++++---- software_scale_context.go | 4 +++- 8 files changed, 70 insertions(+), 26 deletions(-) diff --git a/bit_stream_filter_context.go b/bit_stream_filter_context.go index 5a489dc..eaa6c87 100644 --- a/bit_stream_filter_context.go +++ b/bit_stream_filter_context.go @@ -62,12 +62,14 @@ func (bsfc *BitStreamFilterContext) ReceivePacket(p *Packet) error { func (bsfc *BitStreamFilterContext) Free() { if bsfc.c != nil { // Make sure to clone the classer before freeing the object since - // the C free method resets the pointer + // the C free method may reset the pointer c := newClonedClasser(bsfc) C.av_bsf_free(&bsfc.c) // Make sure to remove from classers after freeing the object since // the C free method may use methods needing the classer - classers.del(c) + if c != nil { + classers.del(c) + } } } diff --git a/class_test.go b/class_test.go index b1d8700..2187085 100644 --- a/class_test.go +++ b/class_test.go @@ -28,7 +28,8 @@ func TestClass(t *testing.T) { func TestClassers(t *testing.T) { cl := len(classers.p) - f := AllocFilterGraph() + f1 := AllocFilterGraph() + f2 := AllocFilterGraph() c := FindDecoder(CodecIDMjpeg) require.NotNil(t, c) bf := FindBitStreamFilterByName("null") @@ -39,7 +40,9 @@ func TestClassers(t *testing.T) { require.NotNil(t, cc) bufferSink := FindFilterByName("buffersink") require.NotNil(t, bufferSink) - fc, err := f.NewFilterContext(bufferSink, "filter_out", nil) + fc1, err := f1.NewFilterContext(bufferSink, "filter_out", nil) + require.NoError(t, err) + _, err = f2.NewFilterContext(bufferSink, "filter_out", nil) require.NoError(t, err) fmc1 := AllocFormatContext() fmc2 := AllocFormatContext() @@ -53,15 +56,16 @@ func TestClassers(t *testing.T) { ssc, err := CreateSoftwareScaleContext(1, 1, PixelFormatRgba, 2, 2, PixelFormatRgba, NewSoftwareScaleContextFlags()) require.NoError(t, err) - require.Equal(t, cl+10, len(classers.p)) - v, ok := classers.get(unsafe.Pointer(f.c)) + require.Equal(t, cl+12, len(classers.p)) + v, ok := classers.get(unsafe.Pointer(f1.c)) require.True(t, ok) - require.Equal(t, f, v) + require.Equal(t, f1, v) bfc.Free() cc.Free() - fc.Free() - f.Free() + fc1.Free() + f1.Free() + f2.Free() fmc1.Free() fmc2.CloseInput() require.NoError(t, ic1.Close()) diff --git a/codec_context.go b/codec_context.go index 9037994..9044944 100644 --- a/codec_context.go +++ b/codec_context.go @@ -45,12 +45,14 @@ func (cc *CodecContext) Free() { } if cc.c != nil { // Make sure to clone the classer before freeing the object since - // the C free method resets the pointer + // the C free method may reset the pointer c := newClonedClasser(cc) C.avcodec_free_context(&cc.c) // Make sure to remove from classers after freeing the object since // the C free method may use methods needing the classer - classers.del(c) + if c != nil { + classers.del(c) + } } } diff --git a/filter_context.go b/filter_context.go index 266282f..fca79ae 100644 --- a/filter_context.go +++ b/filter_context.go @@ -27,10 +27,15 @@ func newFilterContext(c *C.AVFilterContext) *FilterContext { var _ Classer = (*FilterContext)(nil) func (fc *FilterContext) Free() { + // Make sure to clone the classer before freeing the object since + // the C free method may reset the pointer + c := newClonedClasser(fc) C.avfilter_free(fc.c) // Make sure to remove from classers after freeing the object since // the C free method may use methods needing the classer - classers.del(fc) + if c != nil { + classers.del(c) + } } func (fc *FilterContext) BuffersrcAddFrame(f *Frame, fs BuffersrcFlags) error { diff --git a/filter_graph.go b/filter_graph.go index e98509a..b624bcb 100644 --- a/filter_graph.go +++ b/filter_graph.go @@ -10,6 +10,8 @@ import ( // https://github.com/FFmpeg/FFmpeg/blob/n5.0/libavfilter/avfilter.h#L861 type FilterGraph struct { c *C.AVFilterGraph + // We need to store filter contexts to clean classer once filter graph is freed + fcs []*FilterContext } func newFilterGraphFromC(c *C.AVFilterGraph) *FilterGraph { @@ -30,12 +32,23 @@ func AllocFilterGraph() *FilterGraph { func (g *FilterGraph) Free() { if g.c != nil { // Make sure to clone the classer before freeing the object since - // the C free method resets the pointer + // the C free method may reset the pointer c := newClonedClasser(g) + var cfcs []Classer + for _, fc := range g.fcs { + cfcs = append(cfcs, newClonedClasser(fc)) + } C.avfilter_graph_free(&g.c) // Make sure to remove from classers after freeing the object since // the C free method may use methods needing the classer - classers.del(c) + for _, cfc := range cfcs { + if cfc != nil { + classers.del(cfc) + } + } + if c != nil { + classers.del(c) + } } } @@ -85,7 +98,9 @@ func (g *FilterGraph) NewFilterContext(f *Filter, name string, args FilterArgs) if err := newError(C.avfilter_graph_create_filter(&c, f.c, cn, ca, nil, g.c)); err != nil { return nil, err } - return newFilterContext(c), nil + fc := newFilterContext(c) + g.fcs = append(g.fcs, fc) + return fc, nil } func (g *FilterGraph) Parse(content string, inputs, outputs *FilterInOut) error { diff --git a/format_context.go b/format_context.go index dec9c30..c41d2e1 100644 --- a/format_context.go +++ b/format_context.go @@ -51,10 +51,15 @@ func AllocOutputFormatContext(of *OutputFormat, formatName, filename string) (*F } func (fc *FormatContext) Free() { + // Make sure to clone the classer before freeing the object since + // the C free method may reset the pointer + c := newClonedClasser(fc) C.avformat_free_context(fc.c) // Make sure to remove from classers after freeing the object since // the C free method may use methods needing the classer - classers.del(fc) + if c != nil { + classers.del(c) + } } func (fc *FormatContext) BitRate() int64 { @@ -194,18 +199,23 @@ func (fc *FormatContext) OpenInput(url string, fmt *InputFormat, d *Dictionary) } func (fc *FormatContext) CloseInput() { - pb := fc.Pb() if fc.c != nil { // Make sure to clone the classer before freeing the object since - // the C free method resets the pointer + // the C free method may reset the pointer c := newClonedClasser(fc) + var cpb Classer + if pb := fc.Pb(); pb != nil { + cpb = newClonedClasser(pb) + } C.avformat_close_input(&fc.c) // Make sure to remove from classers after freeing the object since // the C free method may use methods needing the classer - if pb != nil { - classers.del(pb) + if cpb != nil { + classers.del(cpb) + } + if c != nil { + classers.del(c) } - classers.del(c) } } diff --git a/io_context.go b/io_context.go index f3ba31d..1156b19 100644 --- a/io_context.go +++ b/io_context.go @@ -124,14 +124,16 @@ func (ic *IOContext) Class() *Class { func (ic *IOContext) Close() error { if ic.c != nil { // Make sure to clone the classer before freeing the object since - // the C free method resets the pointer + // the C free method may reset the pointer c := newClonedClasser(ic) if err := newError(C.avio_closep(&ic.c)); err != nil { return err } // Make sure to remove from classers after freeing the object since // the C free method may use methods needing the classer - classers.del(c) + if c != nil { + classers.del(c) + } } return nil } @@ -146,12 +148,14 @@ func (ic *IOContext) Free() { ic.handlerID = nil } // Make sure to clone the classer before freeing the object since - // the C free method resets the pointer + // the C free method may reset the pointer c := newClonedClasser(ic) C.avio_context_free(&ic.c) // Make sure to remove from classers after freeing the object since // the C free method may use methods needing the classer - classers.del(c) + if c != nil { + classers.del(c) + } } return } diff --git a/software_scale_context.go b/software_scale_context.go index 802108d..d225202 100644 --- a/software_scale_context.go +++ b/software_scale_context.go @@ -66,7 +66,9 @@ func (ssc *SoftwareScaleContext) Free() { C.sws_freeContext(ssc.c) // Make sure to remove from classers after freeing the object since // the C free method may use methods needing the classer - classers.del(c) + if c != nil { + classers.del(c) + } } var _ Classer = (*SoftwareScaleContext)(nil)