| Message ID | 20171108225500.3379-1-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
On 8 November 2017 at 22:55, Michael Niedermayer <michael@niedermayer.cc> wrote: > This gives FFmpeg libs a field that they can freely and safely use. > Avoiding the need of wraping of a users opaque_ref field and its issues. > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavutil/frame.c | 8 +++++++- > libavutil/frame.h | 12 ++++++++++++ > 2 files changed, 19 insertions(+), 1 deletion(-) > > diff --git a/libavutil/frame.c b/libavutil/frame.c > index 982fbb5c81..662a7e5ab5 100644 > --- a/libavutil/frame.c > +++ b/libavutil/frame.c > @@ -383,12 +383,17 @@ FF_ENABLE_DEPRECATION_WARNINGS > #endif > > av_buffer_unref(&dst->opaque_ref); > + av_buffer_unref(&dst->private_ref); > if (src->opaque_ref) { > dst->opaque_ref = av_buffer_ref(src->opaque_ref); > if (!dst->opaque_ref) > return AVERROR(ENOMEM); > } > - > + if (src->private_ref) { > + dst->private_ref = av_buffer_ref(src->private_ref); > + if (!dst->private_ref) > + return AVERROR(ENOMEM); > + } > return 0; > } > > @@ -524,6 +529,7 @@ void av_frame_unref(AVFrame *frame) > av_buffer_unref(&frame->hw_frames_ctx); > > av_buffer_unref(&frame->opaque_ref); > + av_buffer_unref(&frame->private_ref); > > get_frame_defaults(frame); > } > diff --git a/libavutil/frame.h b/libavutil/frame.h > index 0c6aab1c02..7b9bec054a 100644 > --- a/libavutil/frame.h > +++ b/libavutil/frame.h > @@ -563,6 +563,18 @@ typedef struct AVFrame { > /** > * @} > */ > + /** > + * AVBufferRef for free use by libavcodec / libavfilte / .... Code > outside > + * the FFmpeg libs will never > + * check or change the contents of the buffer ref. FFmpeg calls > + * av_buffer_unref() on it when the frame is unreferenced. > + * av_frame_copy_props() calls create a new reference with > av_buffer_ref() > + * for the target frame's private_ref field. > + * > + * The field should be set to NULL by the FFmpeg libs before passing > a frame > + * to the user. > + */ > + AVBufferRef *private_ref; > } AVFrame; > > #if FF_API_FRAME_GET_SET > -- > 2.15.0 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > This makes it sound as if it would get used and can't be overwritten (e.g. feeding frames from a decoder to an encoder and zeroing this field would make things go bad). Just say its temporary storage only and that it doesn't get used after decoding or outputting.
On Wed, Nov 08, 2017 at 11:05:48PM +0000, Rostislav Pehlivanov wrote: > On 8 November 2017 at 22:55, Michael Niedermayer <michael@niedermayer.cc> > wrote: > > > This gives FFmpeg libs a field that they can freely and safely use. > > Avoiding the need of wraping of a users opaque_ref field and its issues. > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavutil/frame.c | 8 +++++++- > > libavutil/frame.h | 12 ++++++++++++ > > 2 files changed, 19 insertions(+), 1 deletion(-) > > > > diff --git a/libavutil/frame.c b/libavutil/frame.c > > index 982fbb5c81..662a7e5ab5 100644 > > --- a/libavutil/frame.c > > +++ b/libavutil/frame.c > > @@ -383,12 +383,17 @@ FF_ENABLE_DEPRECATION_WARNINGS > > #endif > > > > av_buffer_unref(&dst->opaque_ref); > > + av_buffer_unref(&dst->private_ref); > > if (src->opaque_ref) { > > dst->opaque_ref = av_buffer_ref(src->opaque_ref); > > if (!dst->opaque_ref) > > return AVERROR(ENOMEM); > > } > > - > > + if (src->private_ref) { > > + dst->private_ref = av_buffer_ref(src->private_ref); > > + if (!dst->private_ref) > > + return AVERROR(ENOMEM); > > + } > > return 0; > > } > > > > @@ -524,6 +529,7 @@ void av_frame_unref(AVFrame *frame) > > av_buffer_unref(&frame->hw_frames_ctx); > > > > av_buffer_unref(&frame->opaque_ref); > > + av_buffer_unref(&frame->private_ref); > > > > get_frame_defaults(frame); > > } > > diff --git a/libavutil/frame.h b/libavutil/frame.h > > index 0c6aab1c02..7b9bec054a 100644 > > --- a/libavutil/frame.h > > +++ b/libavutil/frame.h > > @@ -563,6 +563,18 @@ typedef struct AVFrame { > > /** > > * @} > > */ > > + /** > > + * AVBufferRef for free use by libavcodec / libavfilte / .... Code > > outside > > + * the FFmpeg libs will never > > + * check or change the contents of the buffer ref. FFmpeg calls > > + * av_buffer_unref() on it when the frame is unreferenced. > > + * av_frame_copy_props() calls create a new reference with > > av_buffer_ref() > > + * for the target frame's private_ref field. > > + * > > + * The field should be set to NULL by the FFmpeg libs before passing > > a frame > > + * to the user. > > + */ > > + AVBufferRef *private_ref; > > } AVFrame; > > > > #if FF_API_FRAME_GET_SET > > -- > > 2.15.0 > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > This makes it sound as if it would get used and can't be overwritten (e.g. > feeding frames from a decoder to an encoder and zeroing this field would > make things go bad). I think i do not understand how you arrive at this from the documentation > > Just say its temporary storage only and that it doesn't get used after > decoding or outputting. Thats would not be correct. this field can be used by libavfilter for example and that also makes it important to clarify who is respnsible for clearing it Because if libavcodec doesnt clear it and its passed into libavfilter which also doesnt clear it that would be bad with one field per lib thats less an issue as there would be no type incompatibility [...]
diff --git a/libavutil/frame.c b/libavutil/frame.c index 982fbb5c81..662a7e5ab5 100644 --- a/libavutil/frame.c +++ b/libavutil/frame.c @@ -383,12 +383,17 @@ FF_ENABLE_DEPRECATION_WARNINGS #endif av_buffer_unref(&dst->opaque_ref); + av_buffer_unref(&dst->private_ref); if (src->opaque_ref) { dst->opaque_ref = av_buffer_ref(src->opaque_ref); if (!dst->opaque_ref) return AVERROR(ENOMEM); } - + if (src->private_ref) { + dst->private_ref = av_buffer_ref(src->private_ref); + if (!dst->private_ref) + return AVERROR(ENOMEM); + } return 0; } @@ -524,6 +529,7 @@ void av_frame_unref(AVFrame *frame) av_buffer_unref(&frame->hw_frames_ctx); av_buffer_unref(&frame->opaque_ref); + av_buffer_unref(&frame->private_ref); get_frame_defaults(frame); } diff --git a/libavutil/frame.h b/libavutil/frame.h index 0c6aab1c02..7b9bec054a 100644 --- a/libavutil/frame.h +++ b/libavutil/frame.h @@ -563,6 +563,18 @@ typedef struct AVFrame { /** * @} */ + /** + * AVBufferRef for free use by libavcodec / libavfilte / .... Code outside + * the FFmpeg libs will never + * check or change the contents of the buffer ref. FFmpeg calls + * av_buffer_unref() on it when the frame is unreferenced. + * av_frame_copy_props() calls create a new reference with av_buffer_ref() + * for the target frame's private_ref field. + * + * The field should be set to NULL by the FFmpeg libs before passing a frame + * to the user. + */ + AVBufferRef *private_ref; } AVFrame; #if FF_API_FRAME_GET_SET
This gives FFmpeg libs a field that they can freely and safely use. Avoiding the need of wraping of a users opaque_ref field and its issues. Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavutil/frame.c | 8 +++++++- libavutil/frame.h | 12 ++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-)