On Tue, Sep 06, 2016 at 05:45:46PM +0200, Hendrik Leppkes wrote:
> 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.

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
From ae128325081392610475b62d0c20165e0cb9536f Mon Sep 17 00:00:00 2001
From: Michael Niedermayer <mich...@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 <mich...@niedermayer.cc>
---
 libavformat/utils.c              | 4 +++-
 tests/ref/fate/copy-trac4914     | 4 ++--
 tests/ref/fate/copy-trac4914-avi | 4 ++--
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/libavformat/utils.c b/libavformat/utils.c
index 7d23c4a..76cbff4 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -3825,8 +3825,10 @@ FF_DISABLE_DEPRECATION_WARNINGS
             st->codec->height = st->internal->avctx->height;
         }
 
-        if (st->codec->codec_tag != MKTAG('t','m','c','d'))
+        if (st->codec->codec_tag != MKTAG('t','m','c','d')) {
             st->codec->time_base = st->internal->avctx->time_base;
+            st->codec->ticks_per_frame = st->internal->avctx->ticks_per_frame;
+        }
         st->codec->framerate = st->avg_frame_rate;
 
         if (st->internal->avctx->subtitle_header) {
diff --git a/tests/ref/fate/copy-trac4914 b/tests/ref/fate/copy-trac4914
index 9301c86..c977f30 100644
--- a/tests/ref/fate/copy-trac4914
+++ b/tests/ref/fate/copy-trac4914
@@ -1,2 +1,2 @@
-84316a64b609052d9974891686fbf607 *tests/data/fate/copy-trac4914.mxf
-566329 tests/data/fate/copy-trac4914.mxf
+8868ae16d99ed03916e9dc7105285471 *tests/data/fate/copy-trac4914.mxf
+560697 tests/data/fate/copy-trac4914.mxf
diff --git a/tests/ref/fate/copy-trac4914-avi b/tests/ref/fate/copy-trac4914-avi
index e23affc..0ee6675 100644
--- a/tests/ref/fate/copy-trac4914-avi
+++ b/tests/ref/fate/copy-trac4914-avi
@@ -1,2 +1,2 @@
-e948f10c90f526ae2c0cf234e1f54128 *tests/data/fate/copy-trac4914-avi.avi
-480886 tests/data/fate/copy-trac4914-avi.avi
+26e4202638bc384b82d2b5eb4d33a5f0 *tests/data/fate/copy-trac4914-avi.avi
+479494 tests/data/fate/copy-trac4914-avi.avi
-- 
2.9.3

Attachment: signature.asc
Description: Digital signature

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to