[FFmpeg-devel] [PATCH] avformat/mov: (v4) fix get_eia608_packet

[Pavel Koshevoy]

On Sun, Feb 23, 2025 at 9:47 AM Pavel Koshevoy <pkoshevoy at gmail.com> wrote:

>
>
> On Fri, Feb 21, 2025 at 9:49 AM Pavel Koshevoy <pkoshevoy at gmail.com>
> wrote:
>
>> If there are no further constructive review comments today and tomorrow
>> then I will commit and push this change on Sunday (if I don't forget).
>>
>> Pavel.
>>
>
>
> Pushed in commit 5021764413a138f340ba46a9b145215b7535ea83
>
> Pavel.
>
>
Cherry-picked to release/4.4, 5.0, 5.1, 6.0, 6.1, 7.0, 7.1

Pavel





>
>
>
>>
>> On Thu, Feb 13, 2025 at 2:22 PM Pavel Koshevoy <pkoshevoy at gmail.com>
>> wrote:
>>
>>> The problem is reproducible with "Test for Quicktime 608 CC file.mov"
>>> from https://samples.ffmpeg.org/MPEG2/subcc/
>>>
>>> ffmpeg -i "Test for Quicktime 608 CC file.mov" -map 0 -c copy -y
>>> remuxed.mov
>>>
>>> Prior to the fix QuickTime Player playback of remuxed.mov would
>>> render garbage text for "English CC" subtitles.
>>> ---
>>>  libavformat/mov.c | 70 +++++++++++++++++++++++++++++++++++++++--------
>>>  1 file changed, 59 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>>> index 85aef33b19..5a91ef5b8c 100644
>>> --- a/libavformat/mov.c
>>> +++ b/libavformat/mov.c
>>> @@ -10788,25 +10788,73 @@ static int mov_change_extradata(AVStream *st,
>>> AVPacket *pkt)
>>>      return 0;
>>>  }
>>>
>>> -static int get_eia608_packet(AVIOContext *pb, AVPacket *pkt, int size)
>>> +static int get_eia608_packet(AVIOContext *pb, AVPacket *pkt, int
>>> src_size)
>>>  {
>>> -    int new_size, ret;
>>> +    /* We can't make assumptions about the structure of the payload,
>>> +       because it may include multiple cdat and cdt2 samples. */
>>> +    const uint32_t cdat = AV_RB32("cdat");
>>> +    const uint32_t cdt2 = AV_RB32("cdt2");
>>> +    int ret, out_size = 0;
>>>
>>> -    if (size <= 8)
>>> +    /* a valid payload must have size, 4cc, and at least 1 byte pair: */
>>> +    if (src_size < 10)
>>>          return AVERROR_INVALIDDATA;
>>> -    new_size = ((size - 8) / 2) * 3;
>>> -    ret = av_new_packet(pkt, new_size);
>>> +
>>> +    /* avoid an int overflow: */
>>> +    if ((src_size - 8) / 2 >= INT_MAX / 3)
>>> +        return AVERROR_INVALIDDATA;
>>> +
>>> +    ret = av_new_packet(pkt, ((src_size - 8) / 2) * 3);
>>>      if (ret < 0)
>>>          return ret;
>>>
>>> -    avio_skip(pb, 8);
>>> -    for (int j = 0; j < new_size; j += 3) {
>>> -        pkt->data[j] = 0xFC;
>>> -        pkt->data[j+1] = avio_r8(pb);
>>> -        pkt->data[j+2] = avio_r8(pb);
>>> +    /* parse and re-format the c608 payload in one pass. */
>>> +    while (src_size >= 10) {
>>> +        const uint32_t atom_size = avio_rb32(pb);
>>> +        const uint32_t atom_type = avio_rb32(pb);
>>> +        const uint32_t data_size = atom_size - 8;
>>> +        const uint8_t cc_field =
>>> +            atom_type == cdat ? 1 :
>>> +            atom_type == cdt2 ? 2 :
>>> +            0;
>>> +
>>> +        /* account for bytes consumed for atom size and type. */
>>> +        src_size -= 8;
>>> +
>>> +        /* make sure the data size stays within the buffer boundaries.
>>> */
>>> +        if (data_size < 2 || data_size > src_size) {
>>> +            ret = AVERROR_INVALIDDATA;
>>> +            break;
>>> +        }
>>> +
>>> +        /* make sure the data size is consistent with N byte pairs. */
>>> +        if (data_size % 2 != 0) {
>>> +            ret = AVERROR_INVALIDDATA;
>>> +            break;
>>> +        }
>>> +
>>> +        if (!cc_field) {
>>> +            /* neither cdat or cdt2 ... skip it */
>>> +            avio_skip(pb, data_size);
>>> +            src_size -= data_size;
>>> +            continue;
>>> +        }
>>> +
>>> +        for (int32_t i = 0; i < data_size; i += 2) {
>>> +            pkt->data[out_size] = (0x1F << 3) | (1 << 2) | (cc_field -
>>> 1);
>>> +            pkt->data[out_size + 1] = avio_r8(pb);
>>> +            pkt->data[out_size + 2] = avio_r8(pb);
>>> +            out_size += 3;
>>> +            src_size -= 2;
>>> +        }
>>>      }
>>>
>>> -    return 0;
>>> +    if (src_size > 0)
>>> +        /* skip any remaining unread portion of the input payload */
>>> +        avio_skip(pb, src_size);
>>> +
>>> +    av_shrink_packet(pkt, out_size);
>>> +    return ret;
>>>  }
>>>
>>>  static int mov_finalize_packet(AVFormatContext *s, AVStream *st,
>>> AVIndexEntry *sample,
>>> --
>>> 2.43.0
>>>
>>>