[MPlayer-dev-eng] [PATCH] improved AIFF PCM Audio support - take 4

[Reimar Döffinger]

Hi,
> > > @@ -167,6 +169,10 @@ int demux_audio_open(demuxer_t* demuxer)
> > >        frmt = fLaC;
> > >        stream_skip(s,-4);
> > >        break;
> > > +    } else if( hdr[0] == 'F' && hdr[1] == 'O' && hdr[2] == 'R' && hdr[3] == 'M' ) {
> > > +      frmt = AIFF;
> > > +      stream_skip(s,-4);
> > > +      break;
> > 
> > I didn't read the patch closely, but is it possible to avoid that
> > backwards-seek? If yes, please do so, because then e.g. playing from
> > stdin will work (even without -cache).
> 
> I used stream_skip(s, -4) to follow the same mental model suggested by the
> existing processing code in demux_audio.c. Currently, WAVE, FLAC and MP3
> all perform backseek to reset the stream and then carry out a full header
> parse. One advantage from this approach is that one no longer needs to
> maintain a separate partial header parser.

That is not (completely) true (and also those 4 bytes are still in the hdr variable). This approach is only necessary when the codec needs the data - and even then it is done in line 348 (except for fLaC, where I was too lazy to fix it). I know I reworked the MP3 part so that it will work without any backwards seeking at all (by skipping a few frames though). The WAVE code only seeks backwards when detection failed, that should rarely happen at that point and I don't think it really matters if that fails... The only case where it is done is for fLaC, and that can't be eliminated (easily) because the codec (stupidly?) needs the data to start with "fLaC".
If that is not the case with your decoder you should be able to avoid
the seeking.

> I feel that if we do choose to eliminate backward-seek we should consider
> doing it globally across all codecs to ensure consistency in both code
> organization and user behavior.

Would be nice, but is a lot of work, especially in a way that will be
accepted (not really such an important feature). At the moment my
strategy is trying to avoid it (when it's not too much work) in new code, keeping everything else for some undetermined moment in the future ;-)

Greetings,
Reimar Döffinger