[FFmpeg-devel] [PATCH] ffmpeg: drop format specific stream copy heuristics

[Michael Niedermayer]

On Tue, Sep 06, 2016 at 06:26:59PM +0200, Michael Niedermayer wrote:
> On Tue, Sep 06, 2016 at 05:45:46PM +0200, Hendrik Leppkes wrote:
> > On Tue, Sep 6, 2016 at 5:10 PM, Michael Niedermayer
> > <michael at niedermayer.cc> wrote:
> > > On Tue, Sep 06, 2016 at 02:39:11PM +0200, Hendrik Leppkes wrote:
> > >> On Tue, Sep 6, 2016 at 2:21 PM, Michael Niedermayer
> > >> <michael at niedermayer.cc> wrote:
> > >> > On Tue, Sep 06, 2016 at 02:18:35PM +0200, Michael Niedermayer wrote:
> > >> >> On Tue, Sep 06, 2016 at 04:57:06AM +0200, Michael Niedermayer wrote:
> > >> >> > On Mon, Sep 05, 2016 at 10:04:35PM -0300, James Almer wrote:
> > >> >> > > On 9/5/2016 12:41 PM, Michael Niedermayer wrote:
> > >> >> > > > On Mon, Sep 05, 2016 at 04:41:52PM +0200, Clément Bœsch wrote:
> > >> >> > > >> From: Clément Bœsch <clement at stupeflix.com>
> > >> >> > > >>
> > >> >> > > >> These adjusted codec fields do not seem to be in use anymore and prevent
> > >> >> > > >> the convert of ffmpeg*.c to codecpar.
> > >> >> > > >
> > >> >> > > >  ./ffmpeg  -i ~/tickets/4914/xdcam8mp2-1s_small.ts -c:v copy out.mxf
> > >> >> > > > fails, no output anymore
> > >> >> > > >
> > >> >> > > > ./ffmpeg -i matrixbench_mpeg2.mpg -c:v copy -t 1 test.avi
> > >> >> > > > the output now has 600fps
> > >> >> > >
> > >> >> > > Even with this code in place the resulting stream in the avi is reported
> > >> >> > > as 100 fps.
> > >> >> >
> > >> >> > that seems to be a regression since
> > >> >> > 6f69f7a8bf6a0d013985578df2ef42ee6b1c7994
> > >> >> >
> > >> >> > IIRC the intended timebase is 1/50 for this kind of content
> > >> >> > (allowing the support of interlaced and field duplicated content to
> > >> >> >  appear later)
> > >> >> >
> > >> >> >
> > >> >> > > And with or without the code, the resulting files play the
> > >> >> > > same with the players i tried.
> > >> >> >
> > >> >> > Higher framerates / finer timebases need noticably more space to
> > >> >> > be stored in avi, thats not the case for other formats and thats
> > >> >> > one reason why avi is treated as a special case.
> > >> >> >
> > >> >> > ill try to look tomorrow why its 100fps since the previous
> > >> >> > codecpar patches. Though 100fps is not nearly as bad as 600fps
> > >> >> > 600 has ~6 times the overhead
> > >> >>
> > >> >> This regression is caused by ticks_per_frame beiing incorrect
> > >> >>
> > >> >> Ill send a patch to fix this
> > >> >
> > >> > patch attached
> > >> >
> > >>
> > >> We don't have time_base in codecpar, so why do we need ticks per frame in it?
> > >
> > > We need both in some form probably
> > > For this regression ticks_per_frame ATM is enough.
> > > For time_base theres code to copy/access it bypassing codecpar
> > >
> > > The way it seems to be currently is that there are fields which
> > > are needed and either they are
> > > in codecpar or
> > > theres some tricks to access them from code sheduled to be removed
> > >  (which will work only as long as the code isnt removed)
> > > or things just dont work.
> > >
> > >
> > >>
> > >> Which time_base does it modify the interpretation of? The field should
> > >> be bundled with that, then.
> > >
> > > the AVCodecContext one, ticks_per_frame is already in AVCodecContext
> > > the AVCodecContext ticks_per_frame though is not exported after codecpar
> > > while the codec timebase still is just not vissibly through codecpar
> > >
> > 
> > Maybe that part should be fixed then, wherever we export that to
> > AVCodecContext, also set its ticks_per_frame properly?
> > It just feels bad to export a property here that standing alone
> > doesn't mean anything.
> > 
> > So fix the export of ticks_per_frame for AVCodecContext, and if later
> > on we decide we really do need time_base in AVCodecParameters, and
> > can't fix whatever needs it differently, we can then include both in
> > there.
> 
> attached
> 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> Dictatorship naturally arises out of democracy, and the most aggravated
> form of tyranny and slavery out of the most extreme liberty. -- Plato

>  libavformat/utils.c              |    4 +++-
>  tests/ref/fate/copy-trac4914     |    4 ++--
>  tests/ref/fate/copy-trac4914-avi |    4 ++--
>  3 files changed, 7 insertions(+), 5 deletions(-)
> 2dc146e807cbdbdbca653a22d827920e8c05b3c8  0001-avformat-Export-ticks_per_frame-in-st-codec.patch
> From ae128325081392610475b62d0c20165e0cb9536f Mon Sep 17 00:00:00 2001
> From: Michael Niedermayer <michael at niedermayer.cc>
> Date: Tue, 6 Sep 2016 18:10:41 +0200
> Subject: [PATCH] avformat: Export ticks_per_frame in st->codec
> 
> Suggested-by: Hendrik Leppkes
> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>

applied

with this we have restored the functionality to prior bug/regression
so it should serve better as a reference.


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you drop bombs on a foreign country and kill a hundred thousand
innocent people, expect your government to call the consequence
"unprovoked inhuman terrorist attacks" and use it to justify dropping
more bombs and killing more people. The technology changed, the idea is old.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160907/99672b12/attachment.sig>