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

[Paul B Mahol]

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.

>
>> +    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.

>
>> +        } 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.

>
>> +            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.

>
>> +            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.