Isaac Richards wrote:
On Thursday 27 January 2005 07:18 pm, Tim Davies wrote:
  
Doesn't handle ignoring the stream later on properly.
      
This is a lot better than a guaranteed segfault.  It does ignore the
stream, but there is no need to abort everything if it can't find a
decoder for *one* audio track.  If no audio streams are present (or
capable of being decoded) then subsequent code *should* deal with this.
A suggestion or some comments as to why it doesn't handle the streams
properly might help.
    

Who's saying that this is limited to audio streams?
  
Not me, but maybe the checks (and the aborting) should be done after the loop.  Have a check that ensures that you found a (decodable) video and audio stream, if that is what is needed.  Borking if it can't deal with one stream is silly.

Is that a better idea?
  
ac3-4.1    - deal with audio codec changes in avformatdecoder
        
The audio codec _can't_ change for a given streamtype.  The startcode of a
stream in a mpeg file is different based on what codec it is.  Streams
returned from libavformat are enumerated by the startcode.  A new codec
will be given a new logical stream.  If this is necessary, something's
badly wrong in the ts2ps junk the dvb code is using.
      
As far as I can see, mpeg.c is not that clever.  Yes, it will try and
put a stream with the same startcode into the same index in nb_streams.
It comes a little unstuck when streams disappear on a channel change,
and then it leaves an "empty" entry in nb_streams (but that is another
issue).
    

Another issue completely.

  
The avformatdecoder.cpp code is not checking for a codec change *within*
a stream anyway.  It is checking for a codec change when it selects a
different stream, and that certainly does happen. A lot!
    

That patch certainly is checking for a codec change within a stream, which as 
stated, cannot happen unless the stream was badly muxed.
  
>From autoSelectAudioTrack:

            if (e->channels > minChannels)
            {
                currentAudioTrack = track;
                wantedAudioStream = tempStream;
                VERBOSE(VB_AUDIO,
                        QString("Auto-selecting audio track #%1 (stream #%2).")
                               .arg(track + 1).arg(tempStream));
                VERBOSE(VB_AUDIO,
                        QString("It has %1 channels and we needed at least %2")
                               .arg(e->channels).arg(minChannels + 1));

                AVCodecContext *e = &ic->streams[wantedAudioStream]->codec;
                CheckAudioParams(e->sample_rate, e->channels, true);
                return true;
            }

Isn't CheckAudioParams running on what could possibly be a new stream?  Which means the codec could change, and therefore avcodec_find_decoder and avcodec_open need to be called in CheckAudioParams?  It actually has to verify that the codec has changed, and not rely on a sample_rate or channels change.  We mostly have Dolby 2.0 which has the same channels and sample_rate as MPEG2 audio.

  
ac3-4.2    - setup GUI for ac3wanted option
ac3-4.3    - make avformatdecoder auto-select AC3 (if "ac3wanted")
        
Won't be applying these, I don't see a need for this option.
      
There are situations when you won't want the AC3 stream, if it is
available.  Of the top of my head, you might have two frontends; one AC3
capable, and one not.
    

You can't have a non-ac3 capable frontend.

  
Well some doofus might not have software AC3 enabled.  Or might not want to use it.  I don't really care myself.

  
And surely if you *can* decode AC3 tracks, you want avformatdecoder to
select that track: otherwise what is the point?  Are you suggesting that
it randomly selects a track, or uses the first one it finds, or what?
    

It should present a list to the user, which it happens to do right now.

  
Does it?  If I don't use the patch, it just uses the first track it finds (in reverse order).

Besides, I don't want a list every time I change channels.  The tracks would change on about half the channel changes here.  Surely it should choose a preferred track, which can then be overridden (with a list or whatever) later.  This is what the set top boxes do.


Tim.
_______________________________________________
mythtv-dev mailing list
[email protected]
http://mythtv.org/cgi-bin/mailman/listinfo/mythtv-dev

Reply via email to