[MPlayer-dev-eng] [PATCH 2/3] Support MPEG streams embedded ingeneric MOV tracks

[Nico Sabbi]

Petr Baudis wrote:

>>when I told you to use demux_rtp.cpp as an example I wished you 
>>would use all
>>the block therein
>>
>>    
>>
>>>+	{
>>>+	// DEMUXER_TYPE_UNKNOWN? I don't know if there can't be other kinds
>>>+	// of MPEG here as well. --pasky
>>>+	demuxer_t* od = demux_open(demuxer->stream, DEMUXER_TYPE_MPEG_PS,
>>>+	                           audio_id, video_id, dvdsub_id, NULL);
>>>+	return new_demuxers_demuxer(od, od, od);
>>>+	}
>>>+    }
>>>      
>>>
>
>I'm sorry, I don't understand - do you mean this block from demux_rtp.c?
>
>  // Hack: If audio and video are demuxed together on a single RTP stream,
>  // then create a new "demuxer_t" structure to allow the higher-level
>  // code to recognize this:
>  if (demux_is_multiplexed_rtp_stream(demuxer)) {
>    stream_t* s = new_ds_stream(demuxer->video);
>    demuxer_t* od = demux_open(s, DEMUXER_TYPE_UNKNOWN,
>                               audio_id, video_id, dvdsub_id, NULL);
>    demuxer = new_demuxers_demuxer(od, od, od);
>  }
>  
>

yes

>The only extra thing there is building an extra stream, but I think
>that is not needed here since we already have a stream. I do not
>understand mplayer internals that well, so perhaps I'm missing
>something...
>  
>

you are taking an unacceptable shortcut, that is ugly and will make bugs
hard to track

>  
>
>>rejected. If you have to make it work so nastily you had better make 
>>mov_check_file()
>>return DEMUXER_TYPE_MPEG_PS; in alternative you can implement this patch 
>>properly
>>and build a proper linked demuxer
>>    
>>
>
>Having this in mov_check_file() would mean that I have to call
>lschunks() from there and parse much larger portion of the MOV file at
>that time, 
>

irrelavant, probing is done only once

>which in turn means that a video and audio track will be
>created at that point, and overally the patch would be a lot more messy,
>  
>

if demux_mov creates an audio and a video track during the probing phase
I don't even want to imagine what else it does...

>I think. However, I still don't see how is the way in the original patch
>nasty, I just set up a new demuxer on the stream. Could you please
>elaborate?
>
>Thanks,
>
>  
>

+	demuxer->stream->start_pos = trak->samples[0].pos;
+	// Hack from demux_rtp.c:
+	{
+	// DEMUXER_TYPE_UNKNOWN? I don't know if there can't be other kinds
+	// of MPEG here as well. --pasky
+	demuxer_t* od = demux_open(demuxer->stream, DEMUXER_TYPE_MPEG_PS,
+	                           audio_id, video_id, dvdsub_id, NULL);
+	return new_demuxers_demuxer(od, od, od);


because with this code you are building a new demuxer on demuxer->stream and keep demux_mov
allocated for nothing.
This is not the way demuxers are supposed to work after they were modularized.
+	}