| Message ID | 20230127211841.59628-1-jpcoiner@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] libavformat/mpegtsenc.c -- correctly re-emit extradata ahead of IDR pictures | expand |
| Context | Check | Description |
|---|---|---|
| andriy/commit_msg_x86 | warning | The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ". |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Fri, 27 Jan 2023, John Coiner wrote: > From: John Coiner <jcoiner@google.com> > > This is the proposed fix for https://trac.ffmpeg.org/ticket/10148 > > Very sorry to send the same patch 3 times; this time with `git send-email' so it should be legal :) > > --- > libavformat/mpegtsenc.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c > index 48d39e6a7d..6cb398f99c 100644 > --- a/libavformat/mpegtsenc.c > +++ b/libavformat/mpegtsenc.c > @@ -1886,17 +1886,21 @@ static int mpegts_write_packet_internal(AVFormatContext *s, AVPacket *pkt) > if (extradd && AV_RB24(st->codecpar->extradata) > 1) > extradd = 0; > > - do { > + while (p < buf_end > + && extradd > 0 > + && (state & 0x1f) != 5 // IDR picture > + && (state & 0x1f) != 1 // non-IDR picture > + ) { > p = avpriv_find_start_code(p, buf_end, &state); > av_log(s, AV_LOG_TRACE, "nal %"PRId32"\n", state & 0x1f); > - if ((state & 0x1f) == 7) > + if ((state & 0x1f) == 7) // SPS NAL > extradd = 0; > - } while (p < buf_end && (state & 0x1f) != 9 && > - (state & 0x1f) != 5 && (state & 0x1f) != 1); > - > - if ((state & 0x1f) != 5) > + } > + if ((state & 0x1f) != 5) { > + // Not an IDR picture > extradd = 0; > - if ((state & 0x1f) != 9) { // AUD NAL > + } > + if (extradd > 0) { This does not look right, because AFAIK you always want to insert an AUD unless it is already there. I guess that is why current code works as it is, it assumes if an AUD is already present, then IDR-s are already prefixed with SPS/PPS, and no magic is needed. I wonder if the issue is indeed caused by this: https://patchwork.ffmpeg.org/project/ffmpeg/patch/tencent_EE0E40DE9A569FE5AB454E6E700A2DA79A08@qq.com/ Because if it is, then maybe nothing need to be fixed in the mpegts muxer. Regards, Marton
diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c index 48d39e6a7d..6cb398f99c 100644 --- a/libavformat/mpegtsenc.c +++ b/libavformat/mpegtsenc.c @@ -1886,17 +1886,21 @@ static int mpegts_write_packet_internal(AVFormatContext *s, AVPacket *pkt) if (extradd && AV_RB24(st->codecpar->extradata) > 1) extradd = 0; - do { + while (p < buf_end + && extradd > 0 + && (state & 0x1f) != 5 // IDR picture + && (state & 0x1f) != 1 // non-IDR picture + ) { p = avpriv_find_start_code(p, buf_end, &state); av_log(s, AV_LOG_TRACE, "nal %"PRId32"\n", state & 0x1f); - if ((state & 0x1f) == 7) + if ((state & 0x1f) == 7) // SPS NAL extradd = 0; - } while (p < buf_end && (state & 0x1f) != 9 && - (state & 0x1f) != 5 && (state & 0x1f) != 1); - - if ((state & 0x1f) != 5) + } + if ((state & 0x1f) != 5) { + // Not an IDR picture extradd = 0; - if ((state & 0x1f) != 9) { // AUD NAL + } + if (extradd > 0) { data = av_malloc(pkt->size + 6 + extradd); if (!data) return AVERROR(ENOMEM);
From: John Coiner <jcoiner@google.com> This is the proposed fix for https://trac.ffmpeg.org/ticket/10148 Very sorry to send the same patch 3 times; this time with `git send-email' so it should be legal :) --- libavformat/mpegtsenc.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)