[FFmpeg-devel] [PATCH] miscelleneous NUT fixes

Diego Biurrun diego
Sun Feb 3 20:38:16 CET 2008


On Sun, Feb 03, 2008 at 08:28:30PM +0100, Michael Niedermayer wrote:
> On Sun, Feb 03, 2008 at 08:25:26PM +0100, Diego Biurrun wrote:
> > On Sun, Feb 03, 2008 at 08:16:39PM +0100, Michael Niedermayer wrote:
> > > On Sun, Feb 03, 2008 at 01:28:03PM -0500, Rich Felker wrote:
> > > > On Sun, Feb 03, 2008 at 12:55:39PM +0100, Michael Niedermayer wrote:
> > > > > On Sun, Feb 03, 2008 at 01:11:33AM -0500, Rich Felker wrote:
> > > > > > On Sun, Feb 03, 2008 at 12:34:46AM +0100, Michael Niedermayer wrote:
> > > > > > > On Sat, Feb 02, 2008 at 02:19:25PM -0500, Rich Felker wrote:
> > > > > > > > On Sat, Feb 02, 2008 at 06:41:20PM +0100, Michael Niedermayer wrote:
> > > > > > > > > > 1. "Bleh, libnut muxed this ;)\n" please, I fixed this bug over a year ago :)
> > > > > > > > > 
> > > > > > > > > rejected, we do not remove workarounds needed for demuxing/decoding
> > > > > > > > 
> > > > > > > > While I agree with this principle, I don't think it applies to
> > > > > > > > under-development formats. The snow decoder does not support every
> > > > > > > > random historical version of snow, and likewise the nut demuxer should
> > > > > > > 
> > > > > > > I would support them if it is easy to do and would not cause a slowdown.
> > > > > > 
> > > > > > I reviewed the code in question and it's actually a bug. Samplerate
> > > > > > num=2 den=3 means 2 samples every 3 seconds, but with the bogus code
> > > > > > in place, it will be interpreted as 3 samples per second. Apparently
> > > > > > lavf does not support non-integer values for samples-per-second so it
> > > > > > should drop an "unsupported format" error in any case rather than
> > > > > > misinterpreting the data.
> > > > > 
> > > > > Maybe we should have disallowed non integer samplerates in nut ...
> > > > 
> > > > No, that creates a fundamental dependency on a physical unit
> > > > (seconds). Also what if one has a stream sampled at 44100 Hz but wants
> > > > to retime the video from NTSC to PAL or film to NTSC?? The conversion
> > > > factor does not divide 44100 evenly so a noninteger samplerate is
> > > > needed.
> > > > 
> > > > > > I stand by Oded's patch to remove the incorrect interpretation and
> > > > > 
> > > > > We have files which need this interpretation, we do not have ones which
> > > > > break due to it. Feel free to add a big comment to the code saying its
> > > > > not spec compliant if you like.
> > > > 
> > > > We have invalid files which should be patched or deleted.
> > > 
> > > Its ffmpegs philosophy to play as many files as possible, broken or not.
> > 
> > I agree with that philosophy, but in this special case of a format that
> > is in the process of being designed I would set a point starting from
> > which all files should be supported.  Remember that NUT files do not
> > exist in the wild yet.  All we would lose is support for a few files on
> > Oded harddrive, if at all.
> 
> And all we would gain is 2 lines of code less

You have rejected patches over 2 lines of code :)

In this special case I think it's not worth even 2 lines of code, people
will be looking at this demuxer as reference implementation after all.

Diego




More information about the ffmpeg-devel mailing list