[FFmpeg-devel] [PATCH] avformat: add NSP demuxer

Paul B Mahol onemda at gmail.com
Mon Dec 4 22:36:02 EET 2017


On 12/4/17, Carl Eugen Hoyos <ceffmpeg at gmail.com> wrote:
> 2017-12-04 17:17 GMT+01:00 Paul B Mahol <onemda at gmail.com>:
>> On 12/3/17, Carl Eugen Hoyos <ceffmpeg at gmail.com> wrote:
>>> 2017-12-01 17:26 GMT+01:00 Paul B Mahol <onemda at gmail.com>:
>>>
>>>> +static int nsp_read_header(AVFormatContext *s)
>>>> +{
>>>> +    int rate = 0, channels = 0;
>>>> +    uint32_t chunk, size;
>>>> +    AVStream *st;
>>>> +    int64_t pos;
>>>> +
>>>> +    avio_skip(s->pb, 12);
>>>
>>> I believe you could set the duration here for the
>>> supported interleaved stereo files.
>>
>> I prefer not.
>
> Isn't this what libavformat normally does?

It auto calculates it from bitrate.

>
>>>> +    st = avformat_new_stream(s, NULL);
>>>> +    if (!st)
>>>> +        return AVERROR(ENOMEM);
>>>> +
>>>> +    while (!avio_feof(s->pb)) {
>>>> +        chunk = avio_rb32(s->pb);
>>>> +        size  = avio_rl32(s->pb);
>>>> +        pos   = avio_tell(s->pb);
>>>> +
>>>> +        if (chunk == MKBETAG('H', 'D', 'R', '8') ||
>>>> +            chunk == MKBETAG('H', 'E', 'D', 'R')) {
>>>> +            if (size < 32)
>>>> +                return AVERROR_INVALIDDATA;
>>>> +            avio_skip(s->pb, 20);
>>>> +            rate = avio_rl32(s->pb);
>>>
>>>> +            avio_skip(s->pb, size - (avio_tell(s->pb) - pos));
>>>
>>> Why is this not "skip(pb, size - 32)" (or whatever the correct
>>> constant is)?
>>
>> To be furure proof.
>
> To a specification change?

No.

>
>>>> +        } else if (chunk == MKBETAG('N', 'O', 'T', 'E')) {
>>>> +            char value[1024];
>>>> +
>>>> +            avio_get_str(s->pb, size, value, sizeof(value));
>>>> +            av_dict_set(&s->metadata, "Note", value, 0);
>>>
>>> Shouldn't this be "comment"?
>>
>> No.
>
> Wouldn't this make the metadata compatible with other containers?

Nope.

>
>>>> +            avio_skip(s->pb, 1);
>>>
>>> Should be something like "avio_skip(pb, size & 1);" according
>>> to the description I found.
>>
>> Changed.
>>
>>>
>>>> +        } else if (chunk == MKBETAG('S', 'D', 'A', 'B')) {
>>>> +            channels = 2;
>>>
>>> If I read correctly, you can set STEREO here.
>>
>> I prefer not.
>
> Are the files not stereo?
> (I am not sure I understand what the format is used for.)

No, container does not store channel layout.

>
>>>> +            break;
>>>
>>>> +        } else if (chunk == MKBETAG('S', 'D', 'A', '_') ||
>>>> +                   chunk == MKBETAG('S', 'D', '_', 'A') ||
>>>> +                   chunk == MKBETAG('S', 'D', '_', '2') ||
>>>> +                   chunk == MKBETAG('S', 'D', '_', '3') ||
>>>> +                   chunk == MKBETAG('S', 'D', '_', '4') ||
>>>> +                   chunk == MKBETAG('S', 'D', '_', '5') ||
>>>> +                   chunk == MKBETAG('S', 'D', '_', '6') ||
>>>> +                   chunk == MKBETAG('S', 'D', '_', '7') ||
>>>> +                   chunk == MKBETAG('S', 'D', '_', '8')) {
>>>
>>> Iiuc, in these cases the file is not read correctly.
>>> If this cannot be implemented easily, a warning should
>>> be shown.
>>
>> I doubt so.
>
> Please correct me:
> If the file is not SDAB, the channels are non-interleaved and
> the chunks follow each other sequentially but are not read
> by your current demuxer, no?

Give me such files first!


More information about the ffmpeg-devel mailing list