2017-12-04 21:36 GMT+01:00 Paul B Mahol <one...@gmail.com>: > On 12/4/17, Carl Eugen Hoyos <ceffm...@gmail.com> wrote: >> 2017-12-04 17:17 GMT+01:00 Paul B Mahol <one...@gmail.com>: >>> On 12/3/17, Carl Eugen Hoyos <ceffm...@gmail.com> wrote: >>>> 2017-12-01 17:26 GMT+01:00 Paul B Mahol <one...@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.
This will fail for multi-channel files and it is always a little less accurate. >>>>> + 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. How can it be future-proof then? >>>>> + } 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. Is there really a container that understands metadata "Note": I did not immediately find one. >>>>> + 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. The specification implies (iirc) that SDAB contains a left and right channel: Isn't this the definition of stereo? >>>>> + 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! I have never seen an nsp file, I only know of this format thanks to you. The only specification I found indicates that for >2 channels, the sound data is not interleaved. Do you read the specification differently? Carl Eugen _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel