| Message ID | 20230118143113.324703-2-tim@ngus.net |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat/assenc: fix incorrect copy of null terminator | expand |
> On Jan 18, 2023, at 22:31, Tim Angus <tim@ngus.net> wrote: > > When writing a subtitle SSA/ASS subtitle file, the > AVCodecParameters::extradata buffer is written directly to the output, > potentially including a null terminating character, which is sometimes > present. The result is the output having a null character in the middle; > this is addressed here by avoiding copying it. > > Signed-off-by: Tim Angus <tim@ngus.net> > --- > libavformat/assenc.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/libavformat/assenc.c b/libavformat/assenc.c > index 1600f0a02b..5e74b84575 100644 > --- a/libavformat/assenc.c > +++ b/libavformat/assenc.c > @@ -69,6 +69,11 @@ static int write_header(AVFormatContext *s) > ass->trailer = trailer; > } > > + /* extradata may or may not be null terminated; in the case where > + * it is, avoid copying a null into the middle of the buffer */ > + while (header_size > 0 && par->extradata[header_size - 1] == '\0') > + header_size--; > + The comment is misleading. extradata is always null terminated, although those paddings don’t count in extradata_size. > avio_write(s->pb, par->extradata, header_size); > if (par->extradata[header_size - 1] != '\n') > avio_write(s->pb, "\r\n", 2); > -- > 2.25.1 > > _______________________________________________ > 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".
On 31/01/2023 12:37, "zhilizhao(赵志立)" wrote: >> + /* extradata may or may not be null terminated; in the case where >> + * it is, avoid copying a null into the middle of the buffer */ >> + while (header_size > 0 && par->extradata[header_size - 1] == '\0') >> + header_size--; >> + > The comment is misleading. extradata is always null terminated, although > those paddings don’t count in extradata_size. That's a bit pedantic, but I take your point. "The contents of extradata may or may..." would be better. Following some discussion on IRC, I've actually submitted another patch that solves the problem in a different way, but I don't think anyone has looked at it yet... http://ffmpeg.org/pipermail/ffmpeg-devel/2023-January/306017.html
diff --git a/libavformat/assenc.c b/libavformat/assenc.c index 1600f0a02b..5e74b84575 100644 --- a/libavformat/assenc.c +++ b/libavformat/assenc.c @@ -69,6 +69,11 @@ static int write_header(AVFormatContext *s) ass->trailer = trailer; } + /* extradata may or may not be null terminated; in the case where + * it is, avoid copying a null into the middle of the buffer */ + while (header_size > 0 && par->extradata[header_size - 1] == '\0') + header_size--; + avio_write(s->pb, par->extradata, header_size); if (par->extradata[header_size - 1] != '\n') avio_write(s->pb, "\r\n", 2);
When writing a subtitle SSA/ASS subtitle file, the AVCodecParameters::extradata buffer is written directly to the output, potentially including a null terminating character, which is sometimes present. The result is the output having a null character in the middle; this is addressed here by avoiding copying it. Signed-off-by: Tim Angus <tim@ngus.net> --- libavformat/assenc.c | 5 +++++ 1 file changed, 5 insertions(+)