Re: [FFmpeg-devel] [PATCH] avcodec/(null|opus)_bsf: Use ff_bsf_get_packet_ref() directly
On 5/8/2020 2:37 PM, Andreas Rheinhardt wrote: > James Almer: >> On 5/8/2020 7:52 AM, Andreas Rheinhardt wrote: >>> Signed-off-by: Andreas Rheinhardt >>> --- >>> The change to the documentation would be unnecessary if James' patch >>> https://ffmpeg.org/pipermail/ffmpeg-devel/2020-April/260984.html were >>> applied. But it is only internal documentation, so changing and >>> reverting it is easy. >> >> How does my patch change the success return values of >> ff_bsf_get_packet_ref()? >> > It doesn't; yet av_bsf_receive_packet() documents that it returns 0 on > success. Values > 0 are against its documentation. Had > ff_bsf_get_packet_ref() ever returned values > 0, then forwarding its > return value without filtering it (as is done currently) would not be > allowed. Therefore this patch aligns the documentation of > ff_bsf_get_packet_ref() with the one from av_bsf_receive_packet(). > Your patch meanwhile would filter any return values > 0 of an > AVBitStreamFilter.filter function away. Even without my patch ff_bsf_get_packet_ref() has no need to return > 0 for any reason, so the change is ok either way (The doxy could also mention EAGAIN and EOF are not failure, btw). Just ensure it's in a separate commit than the bsf changes when you push. > >>> >>> libavcodec/bsf.h | 2 +- >>> libavcodec/null_bsf.c | 7 +-- >>> libavcodec/opus_metadata_bsf.c | 7 +-- >>> 3 files changed, 3 insertions(+), 13 deletions(-) >>> >>> diff --git a/libavcodec/bsf.h b/libavcodec/bsf.h >>> index af035eee44..d04f1d3068 100644 >>> --- a/libavcodec/bsf.h >>> +++ b/libavcodec/bsf.h >>> @@ -35,7 +35,7 @@ int ff_bsf_get_packet(AVBSFContext *ctx, AVPacket **pkt); >>> * @param ctx pointer to AVBSFContext of filter >>> * @param pkt pointer to packet to move reference to >>> * >>> - * @return 0>= on success, negative AVERROR in case of failure >>> + * @return 0 on success, negative AVERROR in case of failure >> >> This change is ok, but I don't think it's relevant to this patch. >> >>> */ >>> int ff_bsf_get_packet_ref(AVBSFContext *ctx, AVPacket *pkt); >>> >>> diff --git a/libavcodec/null_bsf.c b/libavcodec/null_bsf.c >>> index 24d26dfb1a..37f4640c87 100644 >>> --- a/libavcodec/null_bsf.c >>> +++ b/libavcodec/null_bsf.c >>> @@ -24,12 +24,7 @@ >>> #include "avcodec.h" >>> #include "bsf.h" >>> >>> -static int null_filter(AVBSFContext *ctx, AVPacket *pkt) >>> -{ >>> -return ff_bsf_get_packet_ref(ctx, pkt); >>> -} >>> - >>> const AVBitStreamFilter ff_null_bsf = { >>> .name = "null", >>> -.filter = null_filter, >>> +.filter = ff_bsf_get_packet_ref, >>> }; >>> diff --git a/libavcodec/opus_metadata_bsf.c b/libavcodec/opus_metadata_bsf.c >>> index 867ad830d3..d22db54f30 100644 >>> --- a/libavcodec/opus_metadata_bsf.c >>> +++ b/libavcodec/opus_metadata_bsf.c >>> @@ -25,11 +25,6 @@ typedef struct OpusBSFContext { >>> int gain; >>> } OpusBSFContext; >>> >>> -static int opus_metadata_filter(AVBSFContext *bsfc, AVPacket *pkt) >>> -{ >>> -return ff_bsf_get_packet_ref(bsfc, pkt); >>> -} >>> - >>> static int opus_metadata_init(AVBSFContext *bsfc) >>> { >>> OpusBSFContext *s = bsfc->priv_data; >>> @@ -67,6 +62,6 @@ const AVBitStreamFilter ff_opus_metadata_bsf = { >>> .priv_data_size = sizeof(OpusBSFContext), >>> .priv_class = _metadata_class, >>> .init = _metadata_init, >>> -.filter = _metadata_filter, >>> +.filter = _bsf_get_packet_ref, >>> .codec_ids = codec_ids, >>> }; >> >> LGTM otherwise. >> ___ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". >> > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/(null|opus)_bsf: Use ff_bsf_get_packet_ref() directly
James Almer: > On 5/8/2020 7:52 AM, Andreas Rheinhardt wrote: >> Signed-off-by: Andreas Rheinhardt >> --- >> The change to the documentation would be unnecessary if James' patch >> https://ffmpeg.org/pipermail/ffmpeg-devel/2020-April/260984.html were >> applied. But it is only internal documentation, so changing and >> reverting it is easy. > > How does my patch change the success return values of > ff_bsf_get_packet_ref()? > It doesn't; yet av_bsf_receive_packet() documents that it returns 0 on success. Values > 0 are against its documentation. Had ff_bsf_get_packet_ref() ever returned values > 0, then forwarding its return value without filtering it (as is done currently) would not be allowed. Therefore this patch aligns the documentation of ff_bsf_get_packet_ref() with the one from av_bsf_receive_packet(). Your patch meanwhile would filter any return values > 0 of an AVBitStreamFilter.filter function away. >> >> libavcodec/bsf.h | 2 +- >> libavcodec/null_bsf.c | 7 +-- >> libavcodec/opus_metadata_bsf.c | 7 +-- >> 3 files changed, 3 insertions(+), 13 deletions(-) >> >> diff --git a/libavcodec/bsf.h b/libavcodec/bsf.h >> index af035eee44..d04f1d3068 100644 >> --- a/libavcodec/bsf.h >> +++ b/libavcodec/bsf.h >> @@ -35,7 +35,7 @@ int ff_bsf_get_packet(AVBSFContext *ctx, AVPacket **pkt); >> * @param ctx pointer to AVBSFContext of filter >> * @param pkt pointer to packet to move reference to >> * >> - * @return 0>= on success, negative AVERROR in case of failure >> + * @return 0 on success, negative AVERROR in case of failure > > This change is ok, but I don't think it's relevant to this patch. > >> */ >> int ff_bsf_get_packet_ref(AVBSFContext *ctx, AVPacket *pkt); >> >> diff --git a/libavcodec/null_bsf.c b/libavcodec/null_bsf.c >> index 24d26dfb1a..37f4640c87 100644 >> --- a/libavcodec/null_bsf.c >> +++ b/libavcodec/null_bsf.c >> @@ -24,12 +24,7 @@ >> #include "avcodec.h" >> #include "bsf.h" >> >> -static int null_filter(AVBSFContext *ctx, AVPacket *pkt) >> -{ >> -return ff_bsf_get_packet_ref(ctx, pkt); >> -} >> - >> const AVBitStreamFilter ff_null_bsf = { >> .name = "null", >> -.filter = null_filter, >> +.filter = ff_bsf_get_packet_ref, >> }; >> diff --git a/libavcodec/opus_metadata_bsf.c b/libavcodec/opus_metadata_bsf.c >> index 867ad830d3..d22db54f30 100644 >> --- a/libavcodec/opus_metadata_bsf.c >> +++ b/libavcodec/opus_metadata_bsf.c >> @@ -25,11 +25,6 @@ typedef struct OpusBSFContext { >> int gain; >> } OpusBSFContext; >> >> -static int opus_metadata_filter(AVBSFContext *bsfc, AVPacket *pkt) >> -{ >> -return ff_bsf_get_packet_ref(bsfc, pkt); >> -} >> - >> static int opus_metadata_init(AVBSFContext *bsfc) >> { >> OpusBSFContext *s = bsfc->priv_data; >> @@ -67,6 +62,6 @@ const AVBitStreamFilter ff_opus_metadata_bsf = { >> .priv_data_size = sizeof(OpusBSFContext), >> .priv_class = _metadata_class, >> .init = _metadata_init, >> -.filter = _metadata_filter, >> +.filter = _bsf_get_packet_ref, >> .codec_ids = codec_ids, >> }; > > LGTM otherwise. > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/(null|opus)_bsf: Use ff_bsf_get_packet_ref() directly
On 5/8/2020 7:52 AM, Andreas Rheinhardt wrote: > Signed-off-by: Andreas Rheinhardt > --- > The change to the documentation would be unnecessary if James' patch > https://ffmpeg.org/pipermail/ffmpeg-devel/2020-April/260984.html were > applied. But it is only internal documentation, so changing and > reverting it is easy. How does my patch change the success return values of ff_bsf_get_packet_ref()? > > libavcodec/bsf.h | 2 +- > libavcodec/null_bsf.c | 7 +-- > libavcodec/opus_metadata_bsf.c | 7 +-- > 3 files changed, 3 insertions(+), 13 deletions(-) > > diff --git a/libavcodec/bsf.h b/libavcodec/bsf.h > index af035eee44..d04f1d3068 100644 > --- a/libavcodec/bsf.h > +++ b/libavcodec/bsf.h > @@ -35,7 +35,7 @@ int ff_bsf_get_packet(AVBSFContext *ctx, AVPacket **pkt); > * @param ctx pointer to AVBSFContext of filter > * @param pkt pointer to packet to move reference to > * > - * @return 0>= on success, negative AVERROR in case of failure > + * @return 0 on success, negative AVERROR in case of failure This change is ok, but I don't think it's relevant to this patch. > */ > int ff_bsf_get_packet_ref(AVBSFContext *ctx, AVPacket *pkt); > > diff --git a/libavcodec/null_bsf.c b/libavcodec/null_bsf.c > index 24d26dfb1a..37f4640c87 100644 > --- a/libavcodec/null_bsf.c > +++ b/libavcodec/null_bsf.c > @@ -24,12 +24,7 @@ > #include "avcodec.h" > #include "bsf.h" > > -static int null_filter(AVBSFContext *ctx, AVPacket *pkt) > -{ > -return ff_bsf_get_packet_ref(ctx, pkt); > -} > - > const AVBitStreamFilter ff_null_bsf = { > .name = "null", > -.filter = null_filter, > +.filter = ff_bsf_get_packet_ref, > }; > diff --git a/libavcodec/opus_metadata_bsf.c b/libavcodec/opus_metadata_bsf.c > index 867ad830d3..d22db54f30 100644 > --- a/libavcodec/opus_metadata_bsf.c > +++ b/libavcodec/opus_metadata_bsf.c > @@ -25,11 +25,6 @@ typedef struct OpusBSFContext { > int gain; > } OpusBSFContext; > > -static int opus_metadata_filter(AVBSFContext *bsfc, AVPacket *pkt) > -{ > -return ff_bsf_get_packet_ref(bsfc, pkt); > -} > - > static int opus_metadata_init(AVBSFContext *bsfc) > { > OpusBSFContext *s = bsfc->priv_data; > @@ -67,6 +62,6 @@ const AVBitStreamFilter ff_opus_metadata_bsf = { > .priv_data_size = sizeof(OpusBSFContext), > .priv_class = _metadata_class, > .init = _metadata_init, > -.filter = _metadata_filter, > +.filter = _bsf_get_packet_ref, > .codec_ids = codec_ids, > }; LGTM otherwise. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".