[FFmpeg-devel] [PATCH 2/2] libavformat/dsfdec: Error dsf_read_header fixed.

[Michael Niedermayer]

On Sat, Aug 01, 2015 at 11:29:05AM +1000, Peter Ross wrote:
> On Fri, Jul 31, 2015 at 08:56:49PM -0400, Ganesh Ajjanagadde wrote:
> > On Fri, Jul 31, 2015 at 8:08 PM, Michael Niedermayer
> > <michael at niedermayer.cc> wrote:
> > > On Fri, Jul 31, 2015 at 07:33:01PM -0400, Ganesh Ajjanagadde wrote:
> > >> On Fri, Jul 31, 2015 at 7:01 PM, Ihar A. Tumashyk <itumashyk at gmail.com> wrote:
> > >> > Sample rate is written "as is" in header . Is *should not* be devined by
> > >> > 8. Refer spec.:
> > >> > http://dsd-guide.com/sites/default/files/white-
> > >> > papers/DSFFileFormatSpec_E.pdf
> > >> >
> > >> > After this patch ffprobe will corretly show sample rate for DSF files.
> > >> >
> > >> > Signed-off-by: Ihar A. Tumashyk <itumashyk at gmail.com>
> > >> > ---
> > >> >  libavformat/dsfdec.c | 2 +-
> > >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >> >
> > >> > diff --git a/libavformat/dsfdec.c b/libavformat/dsfdec.c
> > >> > index ae198b2..3e162ae 100644
> > >> > --- a/libavformat/dsfdec.c
> > >> > +++ b/libavformat/dsfdec.c
> > >> > @@ -105,7 +105,7 @@ static int dsf_read_header(AVFormatContext *s)
> > >> >
> > >> >      st->codec->codec_type   = AVMEDIA_TYPE_AUDIO;
> > >> >      st->codec->channels     = avio_rl32(pb);
> > >> > -    st->codec->sample_rate  = avio_rl32(pb) / 8;
> > >> > +    st->codec->sample_rate  = avio_rl32(pb);
> > >> >
> > >> >      switch(avio_rl32(pb)) {
> > >> >      case 1: st->codec->codec_id = AV_CODEC_ID_DSD_LSBF_PLANAR; break;
> > >>
> > >> LGTM, thanks for clarifying intent of previous patch.
> > >> I am a little puzzled as to why this was never caught till now,
> > >
> > > Changing the sample rate like in the patch breaks playback (it plays
> > > much too quick)
> > > you can find samples to test at:
> > > http://www.2l.no/hires/
> > 
> > I really need to stop trusting these spec files that much,
> > they are often incomplete as in this case. Ironically,
> > wikipedia here gives a much better description of the format:
> > https://en.wikipedia.org/wiki/Direct_Stream_Digital,
> > where they mention that essentially this signal is oversampled by a factor of 8.
> > As the wiki entry indicates, max frequency of PCM is 352.8 kHz.
> > 
> > In fact, I think previous patch should be reverted:
> > the wikipedia article says that newer format is called "DSD-wide" and
> > uses 8-bit word length.
> > ffprobe yields the downsampled frequency of 352.8 kHz which is what
> > actually gets used when one demuxes/
> > operates on the data per wikipedia.
> > Thus, I believe previous behavior was correct.
> 
> converting between DSD->PCM reduces the sample rate by 8.
> at present, the DSD->PCM conversion happens libavcodec/dsddec.c.
> however, a 'codec' cannot change sample_rate, so the divide by 8 is performed earlier in libavformat.
> 
> yes, this is a hack.
> 
> i have a patch set that moves DSD->PCM conversion into swresample, where it belongs, 

i have nothing against it being in swr, i also have nothing against
it being in the codec, that is iam fine with either.
if some people expressed a preferrance previously maybe you should ask
them directly to ensure you dont spend time updating a patch that
someone then objects against

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes
-------------- 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/20150803/dd9769a6/attachment.sig>