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 <andreas.rheinha...@gmail.com>
>>> ---
>>> 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 = &opus_metadata_class,
>>> .init = &opus_metadata_init,
>>> - .filter = &opus_metadata_filter,
>>> + .filter = &ff_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".
