* [FFmpeg-devel] [PATCH] avformat/mov: (v2) fix get_eia608_packet
@ 2025-02-13 17:55 Pavel Koshevoy
2025-02-13 21:04 ` Pavel Koshevoy
0 siblings, 1 reply; 2+ messages in thread
From: Pavel Koshevoy @ 2025-02-13 17:55 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Pavel Koshevoy
The problem is reproducible with "Test for Quicktime 608 CC file.mov"
from https://samples.ffmpeg.org/MPEG2/subcc/
ffmpeg_g -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 cdt2 atom type tags as part of "English CC" subtitles.
---
libavformat/mov.c | 54 +++++++++++++++++++++++++++++++++++++++--------
1 file changed, 45 insertions(+), 9 deletions(-)
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 85aef33b19..65b4f3dd5c 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -10790,22 +10790,58 @@ static int mov_change_extradata(AVStream *st, AVPacket *pkt)
static int get_eia608_packet(AVIOContext *pb, AVPacket *pkt, int 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. */
+ int ret, out_size_max, out_size = 0;
- if (size <= 8)
+ /* a valide payload must have size, 4cc, and at least 1 byte pair: */
+ if (size <= 10 || size % 2 != 0)
return AVERROR_INVALIDDATA;
- new_size = ((size - 8) / 2) * 3;
- ret = av_new_packet(pkt, new_size);
+
+ /* avoid an int overflow: */
+ if ((size - 8) / 2 >= (INT_MAX - size) / 3)
+ return AVERROR_INVALIDDATA;
+
+ /* Allocate the buffer larger enough to store the output cc_data_pkts
+ and the original c608 payload, safely. */
+ out_size_max = ((size - 8) / 2) * 3;
+ ret = av_new_packet(pkt, out_size_max + size);
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);
+ /* Load the original c608 payload into the last 3rd of the buffer. */
+ if (avio_read(pb, pkt->data + out_size_max, size) != size)
+ return AVERROR_EOF;
+
+ /* parse and re-format the c608 payload in one pass. */
+ for (uint8_t *out = pkt->data, *src = out + out_size_max, *end = src + size; src + 8 <= end; ) {
+ uint32_t atom_size = AV_RB32(src);
+ uint8_t cc_field =
+ memcmp(src + 4, "cdat", 4) == 0 ? 1 :
+ memcmp(src + 4, "cdt2", 4) == 0 ? 2 :
+ 0;
+
+ /* make sure the atom size stays within the buffer boundaries. */
+ if (src + atom_size > end)
+ return AVERROR_INVALIDDATA;
+
+ /* make sure the payload size is consistent with N byte pairs. */
+ if ((atom_size - 8) % 2 != 0)
+ return AVERROR_INVALIDDATA;
+
+ if (cc_field != 0) {
+ for (uint8_t *cc = src + 8, *cc_end = src + atom_size; cc < cc_end; cc += 2) {
+ out[0] = (0x1F << 3) | (1 << 2) | (cc_field - 1);
+ out[1] = cc[0];
+ out[2] = cc[1];
+ out_size += 3;
+ out += 3;
+ }
+ }
+ src += atom_size;
}
+ av_shrink_packet(pkt, out_size);
return 0;
}
--
2.43.0
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat/mov: (v2) fix get_eia608_packet
2025-02-13 17:55 [FFmpeg-devel] [PATCH] avformat/mov: (v2) fix get_eia608_packet Pavel Koshevoy
@ 2025-02-13 21:04 ` Pavel Koshevoy
0 siblings, 0 replies; 2+ messages in thread
From: Pavel Koshevoy @ 2025-02-13 21:04 UTC (permalink / raw)
To: ffmpeg-devel
I've submitted a v3 patch that doesn't do as much over-allocation, similar
to https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2023-June/310456.html
On Thu, Feb 13, 2025 at 10:55 AM Pavel Koshevoy <pkoshevoy@gmail.com> wrote:
> The problem is reproducible with "Test for Quicktime 608 CC file.mov"
> from https://samples.ffmpeg.org/MPEG2/subcc/
>
> ffmpeg_g -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 cdt2 atom type tags as part of "English CC" subtitles.
> ---
> libavformat/mov.c | 54 +++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 45 insertions(+), 9 deletions(-)
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 85aef33b19..65b4f3dd5c 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -10790,22 +10790,58 @@ static int mov_change_extradata(AVStream *st,
> AVPacket *pkt)
>
> static int get_eia608_packet(AVIOContext *pb, AVPacket *pkt, int 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. */
> + int ret, out_size_max, out_size = 0;
>
> - if (size <= 8)
> + /* a valide payload must have size, 4cc, and at least 1 byte pair: */
> + if (size <= 10 || size % 2 != 0)
> return AVERROR_INVALIDDATA;
> - new_size = ((size - 8) / 2) * 3;
> - ret = av_new_packet(pkt, new_size);
> +
> + /* avoid an int overflow: */
> + if ((size - 8) / 2 >= (INT_MAX - size) / 3)
> + return AVERROR_INVALIDDATA;
> +
> + /* Allocate the buffer larger enough to store the output cc_data_pkts
> + and the original c608 payload, safely. */
> + out_size_max = ((size - 8) / 2) * 3;
> + ret = av_new_packet(pkt, out_size_max + size);
> 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);
> + /* Load the original c608 payload into the last 3rd of the buffer. */
> + if (avio_read(pb, pkt->data + out_size_max, size) != size)
> + return AVERROR_EOF;
> +
> + /* parse and re-format the c608 payload in one pass. */
> + for (uint8_t *out = pkt->data, *src = out + out_size_max, *end = src
> + size; src + 8 <= end; ) {
> + uint32_t atom_size = AV_RB32(src);
> + uint8_t cc_field =
> + memcmp(src + 4, "cdat", 4) == 0 ? 1 :
> + memcmp(src + 4, "cdt2", 4) == 0 ? 2 :
> + 0;
> +
> + /* make sure the atom size stays within the buffer boundaries. */
> + if (src + atom_size > end)
> + return AVERROR_INVALIDDATA;
> +
> + /* make sure the payload size is consistent with N byte pairs. */
> + if ((atom_size - 8) % 2 != 0)
> + return AVERROR_INVALIDDATA;
> +
> + if (cc_field != 0) {
> + for (uint8_t *cc = src + 8, *cc_end = src + atom_size; cc <
> cc_end; cc += 2) {
> + out[0] = (0x1F << 3) | (1 << 2) | (cc_field - 1);
> + out[1] = cc[0];
> + out[2] = cc[1];
> + out_size += 3;
> + out += 3;
> + }
> + }
> + src += atom_size;
> }
>
> + av_shrink_packet(pkt, out_size);
> return 0;
> }
>
> --
> 2.43.0
>
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-02-13 21:04 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-13 17:55 [FFmpeg-devel] [PATCH] avformat/mov: (v2) fix get_eia608_packet Pavel Koshevoy
2025-02-13 21:04 ` Pavel Koshevoy
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git