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

[Hendrik Leppkes]

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.

- Hendrik