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

[Måns Rullgård]

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.

-- 
M?ns Rullg?rd
mans at mansr.com