| Message ID | 4cad59ed-2de6-d363-debf-16d3067b379b@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, Nov 04, 2016 at 12:57:39PM -0300, James Almer wrote: > On 11/4/2016 9:43 AM, Michael Niedermayer wrote: > > The bitstream filters do not work with merged in side data > > > > This leaves the input packet split if it is being split. > > It could be merged again, if thats preferred ? That would involve > > an extra malloc and memcpy though > > Since side data is being split and merged at a latter point during > av_interleaved_write_frame() and av_write_frame() calls, maybe we > could just move the relevant code before the autobsf kicks in. > > See attached patch. FATE passes and the sample from ticket 5927 > works with autobsf, same as with your patch. > It doesn't change ffmpeg.c manual bsf's behavior, though. For that > your commit from yesterday is still needed. > > mux.c | 23 ++++++++++++++--------- > 1 file changed, 14 insertions(+), 9 deletions(-) > e21140de065a58aec07596fb2e16dcdfed45e920 0001-avformat-mux-split-side-data-earlier-in-av_write_fra.patch > From 44fbcbd855d259126d889e5281e64c8988e57f18 Mon Sep 17 00:00:00 2001 > From: James Almer <jamrial@gmail.com> > Date: Fri, 4 Nov 2016 12:48:20 -0300 > Subject: [PATCH] avformat/mux: split side data earlier in av_write_frame and > av_interleaved_write_frame > > Similarly, merge it back before returning. > > This fixes ticket #5927. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavformat/mux.c | 23 ++++++++++++++--------- > 1 file changed, 14 insertions(+), 9 deletions(-) should be ok i think [...] thx
On 11/4/2016 3:12 PM, Michael Niedermayer wrote: > On Fri, Nov 04, 2016 at 12:57:39PM -0300, James Almer wrote: >> On 11/4/2016 9:43 AM, Michael Niedermayer wrote: >>> The bitstream filters do not work with merged in side data >>> >>> This leaves the input packet split if it is being split. >>> It could be merged again, if thats preferred ? That would involve >>> an extra malloc and memcpy though >> >> Since side data is being split and merged at a latter point during >> av_interleaved_write_frame() and av_write_frame() calls, maybe we >> could just move the relevant code before the autobsf kicks in. >> >> See attached patch. FATE passes and the sample from ticket 5927 >> works with autobsf, same as with your patch. >> It doesn't change ffmpeg.c manual bsf's behavior, though. For that >> your commit from yesterday is still needed. >> > >> mux.c | 23 ++++++++++++++--------- >> 1 file changed, 14 insertions(+), 9 deletions(-) >> e21140de065a58aec07596fb2e16dcdfed45e920 0001-avformat-mux-split-side-data-earlier-in-av_write_fra.patch >> From 44fbcbd855d259126d889e5281e64c8988e57f18 Mon Sep 17 00:00:00 2001 >> From: James Almer <jamrial@gmail.com> >> Date: Fri, 4 Nov 2016 12:48:20 -0300 >> Subject: [PATCH] avformat/mux: split side data earlier in av_write_frame and >> av_interleaved_write_frame >> >> Similarly, merge it back before returning. >> >> This fixes ticket #5927. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavformat/mux.c | 23 ++++++++++++++--------- >> 1 file changed, 14 insertions(+), 9 deletions(-) > > should be ok i think > > [...] > > thx Pushed.
On 11/4/2016 3:12 PM, Michael Niedermayer wrote: > On Fri, Nov 04, 2016 at 12:57:39PM -0300, James Almer wrote: >> On 11/4/2016 9:43 AM, Michael Niedermayer wrote: >>> The bitstream filters do not work with merged in side data >>> >>> This leaves the input packet split if it is being split. >>> It could be merged again, if thats preferred ? That would involve >>> an extra malloc and memcpy though >> >> Since side data is being split and merged at a latter point during >> av_interleaved_write_frame() and av_write_frame() calls, maybe we >> could just move the relevant code before the autobsf kicks in. >> >> See attached patch. FATE passes and the sample from ticket 5927 >> works with autobsf, same as with your patch. >> It doesn't change ffmpeg.c manual bsf's behavior, though. For that >> your commit from yesterday is still needed. >> > >> mux.c | 23 ++++++++++++++--------- >> 1 file changed, 14 insertions(+), 9 deletions(-) >> e21140de065a58aec07596fb2e16dcdfed45e920 0001-avformat-mux-split-side-data-earlier-in-av_write_fra.patch >> From 44fbcbd855d259126d889e5281e64c8988e57f18 Mon Sep 17 00:00:00 2001 >> From: James Almer <jamrial@gmail.com> >> Date: Fri, 4 Nov 2016 12:48:20 -0300 >> Subject: [PATCH] avformat/mux: split side data earlier in av_write_frame and >> av_interleaved_write_frame >> >> Similarly, merge it back before returning. >> >> This fixes ticket #5927. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavformat/mux.c | 23 ++++++++++++++--------- >> 1 file changed, 14 insertions(+), 9 deletions(-) > > should be ok i think > > [...] > > thx Looking at it again, it seems correct for av_write_frame() but not for av_interleaved_write_frame(), where side data is being merged right before the packet gets unref'd. I'm going to revert my commit and apply your patch instead. Even if i try to fix my commit, it's more complexity for no apparent gain i think. Any objections?
On Fri, Nov 04, 2016 at 07:59:48PM -0300, James Almer wrote: > On 11/4/2016 3:12 PM, Michael Niedermayer wrote: > > On Fri, Nov 04, 2016 at 12:57:39PM -0300, James Almer wrote: > >> On 11/4/2016 9:43 AM, Michael Niedermayer wrote: > >>> The bitstream filters do not work with merged in side data > >>> > >>> This leaves the input packet split if it is being split. > >>> It could be merged again, if thats preferred ? That would involve > >>> an extra malloc and memcpy though > >> > >> Since side data is being split and merged at a latter point during > >> av_interleaved_write_frame() and av_write_frame() calls, maybe we > >> could just move the relevant code before the autobsf kicks in. > >> > >> See attached patch. FATE passes and the sample from ticket 5927 > >> works with autobsf, same as with your patch. > >> It doesn't change ffmpeg.c manual bsf's behavior, though. For that > >> your commit from yesterday is still needed. > >> > > > >> mux.c | 23 ++++++++++++++--------- > >> 1 file changed, 14 insertions(+), 9 deletions(-) > >> e21140de065a58aec07596fb2e16dcdfed45e920 0001-avformat-mux-split-side-data-earlier-in-av_write_fra.patch > >> From 44fbcbd855d259126d889e5281e64c8988e57f18 Mon Sep 17 00:00:00 2001 > >> From: James Almer <jamrial@gmail.com> > >> Date: Fri, 4 Nov 2016 12:48:20 -0300 > >> Subject: [PATCH] avformat/mux: split side data earlier in av_write_frame and > >> av_interleaved_write_frame > >> > >> Similarly, merge it back before returning. > >> > >> This fixes ticket #5927. > >> > >> Signed-off-by: James Almer <jamrial@gmail.com> > >> --- > >> libavformat/mux.c | 23 ++++++++++++++--------- > >> 1 file changed, 14 insertions(+), 9 deletions(-) > > > > should be ok i think > > > > [...] > > > > thx > > Looking at it again, it seems correct for av_write_frame() but not for > av_interleaved_write_frame(), where side data is being merged right before > the packet gets unref'd. > I'm going to revert my commit and apply your patch instead. Even if i try > to fix my commit, it's more complexity for no apparent gain i think. > > Any objections? no objections [...]
On 11/4/2016 9:54 PM, Michael Niedermayer wrote: > On Fri, Nov 04, 2016 at 07:59:48PM -0300, James Almer wrote: >> On 11/4/2016 3:12 PM, Michael Niedermayer wrote: >>> On Fri, Nov 04, 2016 at 12:57:39PM -0300, James Almer wrote: >>>> On 11/4/2016 9:43 AM, Michael Niedermayer wrote: >>>>> The bitstream filters do not work with merged in side data >>>>> >>>>> This leaves the input packet split if it is being split. >>>>> It could be merged again, if thats preferred ? That would involve >>>>> an extra malloc and memcpy though >>>> >>>> Since side data is being split and merged at a latter point during >>>> av_interleaved_write_frame() and av_write_frame() calls, maybe we >>>> could just move the relevant code before the autobsf kicks in. >>>> >>>> See attached patch. FATE passes and the sample from ticket 5927 >>>> works with autobsf, same as with your patch. >>>> It doesn't change ffmpeg.c manual bsf's behavior, though. For that >>>> your commit from yesterday is still needed. >>>> >>> >>>> mux.c | 23 ++++++++++++++--------- >>>> 1 file changed, 14 insertions(+), 9 deletions(-) >>>> e21140de065a58aec07596fb2e16dcdfed45e920 0001-avformat-mux-split-side-data-earlier-in-av_write_fra.patch >>>> From 44fbcbd855d259126d889e5281e64c8988e57f18 Mon Sep 17 00:00:00 2001 >>>> From: James Almer <jamrial@gmail.com> >>>> Date: Fri, 4 Nov 2016 12:48:20 -0300 >>>> Subject: [PATCH] avformat/mux: split side data earlier in av_write_frame and >>>> av_interleaved_write_frame >>>> >>>> Similarly, merge it back before returning. >>>> >>>> This fixes ticket #5927. >>>> >>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>> --- >>>> libavformat/mux.c | 23 ++++++++++++++--------- >>>> 1 file changed, 14 insertions(+), 9 deletions(-) >>> >>> should be ok i think >>> >>> [...] >>> >>> thx >> >> Looking at it again, it seems correct for av_write_frame() but not for >> av_interleaved_write_frame(), where side data is being merged right before >> the packet gets unref'd. >> I'm going to revert my commit and apply your patch instead. Even if i try >> to fix my commit, it's more complexity for no apparent gain i think. >> >> Any objections? > > no objections Done. Thanks, and sorry for the noise.
From 44fbcbd855d259126d889e5281e64c8988e57f18 Mon Sep 17 00:00:00 2001 From: James Almer <jamrial@gmail.com> Date: Fri, 4 Nov 2016 12:48:20 -0300 Subject: [PATCH] avformat/mux: split side data earlier in av_write_frame and av_interleaved_write_frame Similarly, merge it back before returning. This fixes ticket #5927. Signed-off-by: James Almer <jamrial@gmail.com> --- libavformat/mux.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/libavformat/mux.c b/libavformat/mux.c index 2dac381..816e856 100644 --- a/libavformat/mux.c +++ b/libavformat/mux.c @@ -692,7 +692,7 @@ FF_ENABLE_DEPRECATION_WARNINGS */ static int write_packet(AVFormatContext *s, AVPacket *pkt) { - int ret, did_split; + int ret; int64_t pts_backup, dts_backup; pts_backup = pkt->pts; @@ -755,8 +755,6 @@ static int write_packet(AVFormatContext *s, AVPacket *pkt) } } - did_split = av_packet_split_side_data(pkt); - if (!s->internal->header_written) { ret = s->internal->write_header_ret ? s->internal->write_header_ret : write_header_internal(s); if (ret < 0) @@ -780,9 +778,6 @@ static int write_packet(AVFormatContext *s, AVPacket *pkt) } fail: - if (did_split) - av_packet_merge_side_data(pkt); - if (ret < 0) { pkt->pts = pts_backup; pkt->dts = dts_backup; @@ -918,7 +913,7 @@ static int do_packet_auto_bsf(AVFormatContext *s, AVPacket *pkt) { int av_write_frame(AVFormatContext *s, AVPacket *pkt) { - int ret; + int ret, did_split = 0; ret = prepare_input_packet(s, pkt); if (ret < 0) @@ -939,7 +934,8 @@ int av_write_frame(AVFormatContext *s, AVPacket *pkt) return ret; } return 1; - } + } else + did_split = av_packet_split_side_data(pkt); ret = do_packet_auto_bsf(s, pkt); if (ret <= 0) @@ -958,6 +954,10 @@ int av_write_frame(AVFormatContext *s, AVPacket *pkt) if (ret >= 0) s->streams[pkt->stream_index]->nb_frames++; + + if (did_split) + av_packet_merge_side_data(pkt); + return ret; } @@ -1224,7 +1224,7 @@ static int interleave_packet(AVFormatContext *s, AVPacket *out, AVPacket *in, in int av_interleaved_write_frame(AVFormatContext *s, AVPacket *pkt) { - int ret, flush = 0; + int ret, did_split = 0, flush = 0; ret = prepare_input_packet(s, pkt); if (ret < 0) @@ -1233,6 +1233,8 @@ int av_interleaved_write_frame(AVFormatContext *s, AVPacket *pkt) if (pkt) { AVStream *st = s->streams[pkt->stream_index]; + did_split = av_packet_split_side_data(pkt); + ret = do_packet_auto_bsf(s, pkt); if (ret == 0) return 0; @@ -1280,6 +1282,9 @@ int av_interleaved_write_frame(AVFormatContext *s, AVPacket *pkt) return s->pb->error; } fail: + if (did_split) + av_packet_merge_side_data(pkt); + av_packet_unref(pkt); return ret; } -- 2.10.1