On Tue, Sep 6, 2016 at 5:10 PM, Michael Niedermayer <mich...@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 >> <mich...@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 <clem...@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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
