Justin Ruggles <[email protected]> added the comment:

Reimar Döffinger wrote:

> Reimar Döffinger <[email protected]> added the comment:
> 
> On Sat, Oct 10, 2009 at 05:50:50PM +0000, Justin Ruggles wrote:
>> --- a/libavformat/aiff.c
>> +++ b/libavformat/aiff.c
>> @@ -46,6 +46,12 @@ static const AVCodecTag codec_aiff_tags[] = {
>>  #define AIFF                    0
>>  #define AIFF_C_VERSION1         0xA2805140
>>  
>> +typedef struct {
>> +    int64_t data_offset;
> 
> I'd expect you could reuse AVFormatContext->data_offset, unless you want
> to support multiple SSND chunks but I expect the current code not to
> work well for that anyway...

Ah, I didn't know about AVFormatContext->data_offset.  That would work
much better.

I didn't know multiple SSND chunks were possible.  I'll have a look at
how the WAV demuxer handles multiple DATA chunks and see if I can use a
similar solution for AIFF.

>> @@ -368,7 +375,9 @@ static int aiff_read_header(AVFormatContext *s,
>>          case MKTAG('S', 'S', 'N', 'D'):     /* Sampled sound chunk */
>>              offset = get_be32(pb);      /* Offset of sound data */
>>              get_be32(pb);               /* BlockSize... don't care */
>> +            ctx->data_size = size - (8 + offset);
>>              offset += url_ftell(pb);    /* Compute absolute data offset */
>> +            ctx->data_offset = offset;
>>              if (st->codec->block_align)    /* Assume COMM already parsed */
>>                  goto got_sound;
>>              if (url_is_streamed(pb)) {
>> @@ -420,10 +429,23 @@ static int aiff_read_packet(AVFormatContext *s,
>>                              AVPacket *pkt)
>>  {
>>      AVStream *st = s->streams[0];
>> +    AIFFInputContext *ctx = s->priv_data;
>> +    int64_t max_size, pos;
>>      int res;
>>  
>> +    /* calculate size of remaining data */
>> +    pos = url_ftell(s->pb);
>> +    if (pos < ctx->data_offset) {
>> +        url_fseek(s->pb, ctx->data_offset, SEEK_SET);
>> +        pos = ctx->data_offset;
>> +    }
>> +    max_size = ctx->data_size - (pos - ctx->data_offset);
>> +    if (max_size <= 0)
>> +        return AVERROR_EOF;
> 
> Why not just have a single data_end field?

Sure, that will work too.

> And what is the purpose of the pos < ctx->data_offset check anyway?

It was to handle the case when the file is seeked to before the start of
data.  But if AVFormatContext->data_offset is used as you suggested,
this should be handled in pcm_read_seek().

> Also I wonder if/how well this works with multiple SSND chunks and
> seeking (probably not that important, but a nice-to-have).

Right now it doesn't work for demuxing, much less seeking, with multiple
SSND chunks.  The patch does not change that.  But yes, it should be
fixed to handle it properly.  Now if I only had a sample...

_____________________________________________________
FFmpeg issue tracker <[email protected]>
<https://roundup.ffmpeg.org/roundup/ffmpeg/issue1455>
_____________________________________________________

Reply via email to