[FFmpeg-devel] [PATCH 2/4] avcodec/ccaption_dec: check the length of packet and return used length

[Andreas Rheinhardt]

Andreas Rheinhardt:
> lance.lmwang at gmail.com:
>> On Thu, May 12, 2022 at 08:25:29AM +0200, Paul B Mahol wrote:
>>> On Thu, May 12, 2022 at 1:39 AM <lance.lmwang at gmail.com> wrote:
>>>
>>>> On Wed, May 11, 2022 at 09:47:52PM +0200, Paul B Mahol wrote:
>>>>> why?
>>>>
>>>> assuming the len is 1, the following code will access the next 3
>>>> array anymore, I think it's better to check the i with len -2.
>>>>
>>>> for (i = 0; i < len; i += 3) {
>>>> to
>>>> for (i = 0; i < len - 2; i += 3) {
>>>>
>>>> for the return, I think it's correct to return the used length,
>>>> if it's error, have return already. right?
>>>
>>>
>>> I doubt so.
>>
>> maybe I'm misunderstand it, but from the comments, the API claims that:
>> libavcodec/codec_internal.h
>> 175          * @return amount of bytes read from the packet on success,
>> 176          *         negative error code on failure
>> 177          */
>> 178         int (*decode)(struct AVCodecContext *avctx, struct AVFrame *frame,
>> 179                       int *got_frame_ptr, struct AVPacket *avpkt);
>> 180         /**
>> 181          * Decode subtitle data to an AVSubtitle.
>> 182          * cb is in this state if cb_type is FF_CODEC_CB_TYPE_DECODE_SUB.
>> 183          *
>> 184          * Apart from that this is like the decode callback.
>> 185          */
>> 186         int (*decode_sub)(struct AVCodecContext *avctx, struct AVSubtitle *sub,
>> 187                           int *got_frame_ptr, const struct AVPacket *avpkt);
>>
> 
> This is correct. It is not only the internal API which claims that, but
> the public API, too. And this just not honoured, in particular not in
> case of subtitle recoding. See
> https://github.com/mkver/FFmpeg/commit/ba1564c532654888015d67b70bf73d117c2d9f75
> 

It seems like there really are people who call this in a loop until all
the input is exhausted (as the documentation leads you to believe (The
internal uses of avcodec_decode_subtitle2 don't do this.)):
https://github.com/HandBrake/HandBrake/blob/a40fb6bce6755209461166140f131f26a2857eb9/libhb/decavsub.c#L335
I wonder if they ever got something meaningful the second time they
called it with the same packet (presuming there was a second time).

- Andreas