[FFmpeg-devel] [PATCH] dvd audio sub-stream handling in the mpeg2 demuxer

[Jai Menon]

On Wed, Feb 11, 2009 at 3:01 PM, M?ns Rullg?rd <mans at mansr.com> wrote:
> Jai Menon <jmenon86 at gmail.com> writes:
>
>> On Tue, Feb 10, 2009 at 5:03 PM, M?ns Rullg?rd <mans at mansr.com> wrote:
>>> Michael Niedermayer <michaelni at gmx.at> writes:
>>>
>>>> On Tue, Feb 10, 2009 at 11:39:24AM +0530, Jai Menon wrote:
>>>>> On Tue, Feb 10, 2009 at 6:23 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
>>>>> > On Mon, Feb 09, 2009 at 10:02:51AM +0530, Jai Menon wrote:
>>>>> >> Hi,
>>>>> >>
>>>>> >> Attached patch allows for demuxing of audio substreams stored as 0x06 type.
>>>>> >> Raw a/52 and subtitle detection is not included because of lack of a sample.
>>>>> >> Fixes issue 725 : MPEG2 PS with PCM audio
>>>>> >>
>>>>> >> --
>>>>> >> Regards,
>>>>> >>
>>>>> >> Jai
>>>>> >
>>>>> >> Index: libavformat/mpeg.c
>>>>> >> ===================================================================
>>>>> >> --- libavformat/mpeg.c        (revision 16998)
>>>>> >> +++ libavformat/mpeg.c        (working copy)
>>>>> >> @@ -450,6 +450,23 @@
>>>>> >>          } else if(es_type == STREAM_TYPE_AUDIO_AC3){
>>>>> >>              codec_id = CODEC_ID_AC3;
>>>>> >>              type = CODEC_TYPE_AUDIO;
>>>>> >> +        } else if(es_type == STREAM_TYPE_PRIVATE_DATA){
>>>>> >> +            uint8_t audio_id;
>>>>> >> +
>>>>> >> +            // probe for dvd audio sub-stream
>>>>> >> +            type = CODEC_TYPE_AUDIO;
>>>>> >> +            audio_id = get_byte(s->pb);
>>>>> >> +            url_fseek(s->pb, -1, SEEK_CUR);
>>>>> >> +            switch(audio_id & 0xe0) {
>>>>> >> +                case 0xa0:  codec_id = CODEC_ID_PCM_DVD;
>>>>> >> +                            break;
>>>>> >> +                case 0x80:  if((audio_id & 0xf8) == 0x88)
>>>>> >> +                                 codec_id = CODEC_ID_DTS;
>>>>> >> +                            else codec_id = CODEC_ID_AC3;
>>>>> >> +                            break;
>>>>> >> +                default:    av_log(s, AV_LOG_ERROR, "Unknown 0x1bd sub-stream\n");
>>>>> >> +                            goto skip;
>>>>> >> +            }
>>>>> >>          } else {
>>>>> >>              goto skip;
>>>>> >>          }
>>>>> >> @@ -508,7 +525,17 @@
>>>>> >>   found:
>>>>> >>      if(st->discard >= AVDISCARD_ALL)
>>>>> >>          goto skip;
>>>>> >> -    if (startcode >= 0xa0 && startcode <= 0xaf) {
>>>>> >> +
>>>>> >> +    if(startcode == 0x1bd) {
>>>>> >> +        // skip dvd audio sub-stream header
>>>>> >> +        url_fskip(s->pb, 4);
>>>>> >> +        len -= 4;
>>>>> >> +    }
>>>>> >> +
>>>>> >> +    if ((startcode >= 0xa0 && startcode <= 0xaf) ||
>>>>> >> +        ((startcode == 0x1bd) &&
>>>>> >> +        ((st->codec->codec_id == CODEC_ID_PCM_S16BE) ||
>>>>> >> +         (st->codec->codec_id == CODEC_ID_PCM_DVD)))) {
>>>>> >>          int b1, freq;
>>>>> >>
>>>>> >>          /* for LPCM, we just skip the header and consider it is raw
>>>>> >
>>>>> > what about:
>>>>> >     }
>>>>> >
>>>>> >     es_type = m->psm_es_type[startcode & 0xff];
>>>>> > -    if(es_type > 0){
>>>>> > +    if(es_type > 0 && es_type != STREAM_TYPE_PRIVATE_DATA){
>>>>> >         if(es_type == STREAM_TYPE_VIDEO_MPEG1){
>>>>> >             codec_id = CODEC_ID_MPEG2VIDEO;
>>>>> >             type = CODEC_TYPE_VIDEO;
>>>>> > ...
>>>>> >
>>>>> > and then have the detection under if(startcode == 0x1bd)
>>>>> > this seems more robust but maybe iam missing something?
>>>>>
>>>>> yes thats better, i was just trying hard not to change the structure
>>>>> of the existing code.
>>>>>
>>>>> > also id read the 4 byte header before
>>>>> >  /* now find stream */
>>>>> >    for(i=0;i<s->nb_streams;i++) {
>>>>> >        st = s->streams[i];
>>>>> >        if (st->id == startcode)
>>>>> >            goto found;
>>>>> >    }
>>>>> > that would avoid the seek back and "skip dvd audio sub-stream header"
>>>>>
>>>>> reworked and revised patch attached.
>>>>
>>>> looks ok assuming mans has no sugestions for improvment
>>>
>>> No immediate ideas here.  The entire stream type detection code could
>>> use a cleanup, but that's not Jai's job.
>>>
>>
>> So is this okay to apply then?
>
> No objections from me, so I think you can go ahead.

In that case, could somebody please commit.
Thanks.

-- 
Regards,

Jai