[PATCH] cdxl: fix duration
fixes ticket #1937
On 17/12/2012 6:55 AM, Piotr Bandurski wrote:
fixes ticket #1937
[...]
@@ -129,6 +132,8 @@ static int cdxl_read_packet(AVFormatContext *s, AVPacket *pkt) audio_size = AV_RB16(&cdxl->header[22]); image_size = FFALIGN(width, 16) * height * cdxl->header[19] / 8; video_size = palette_size + image_size; + + frames = cdxl->filesize / (audio_size + video_size);
This seems kind of error prone to me. It doesn't account for things like header, palette size, and such, I think.
@@ -175,6 +180,10 @@ static int cdxl_read_packet(AVFormatContext *s, AVPacket *pkt) st->codec->codec_id = AV_CODEC_ID_CDXL; st->codec->width = width; st->codec->height = height; + if(cdxl->framerate) + st->duration = frames; + else + st->duration = frames * audio_size;
Needs to be properly indented. - Derek
On 17/12/2012 12:11 PM, Derek Buitenhuis wrote:
+ st->duration = frames * audio_size;
Actually, how does this even work? Audio size is not the number of total samples, AFAIK. - Derek
Hi,
On 17/12/2012 12:11 PM, Derek Buitenhuis wrote:
+ st->duration = frames * audio_size;
Actually, how does this even work? Audio size is not the number of total samples, AFAIK.
Generally it should be frames * audio_size / cdxl->sample_rate, but FFmpeg divides by sample rate automaticly because of avpriv_set_pts_info(st, 64, 1, cdxl->sample_rate); Regards
On 17/12/2012 2:46 PM, Piotr Bandurski wrote:
Generally it should be frames * audio_size / cdxl->sample_rate, but FFmpeg divides by sample rate automaticly because of
For that to work, frames * audios_size would have to equal the total number of samples. I don't really see how you get that. - Derek
Hi,
@@ -129,6 +132,8 @@ static int cdxl_read_packet(AVFormatContext *s, AVPacket *pkt) audio_size = AV_RB16(&cdxl->header[22]); image_size = FFALIGN(width, 16) * height * cdxl->header[19] / 8; video_size = palette_size + image_size; + + frames = cdxl->filesize / (audio_size + video_size);
This seems kind of error prone to me. It doesn't account for things like header, palette size, and such, I think.
Maybe, but it seems to work ok on the files I've tested, so if there are some errors then not big enough to be noticable in the final duration, but if it's possible to make a better calculation for number of frames then please suggest a code I could use and I will do so.
@@ -175,6 +180,10 @@ static int cdxl_read_packet(AVFormatContext *s, AVPacket *pkt) st->codec->codec_id = AV_CODEC_ID_CDXL; st->codec->width = width; st->codec->height = height; + if(cdxl->framerate) + st->duration = frames; + else + st->duration = frames * audio_size;
Needs to be properly indented.
Thanks, fixed. Regards
On Mon, Dec 17, 2012 at 12:55:34PM +0100, Piotr Bandurski wrote:
fixes ticket #1937
diff --git a/libavformat/cdxl.c b/libavformat/cdxl.c index 185b745..5f608bf 100644 --- a/libavformat/cdxl.c +++ b/libavformat/cdxl.c @@ -37,6 +37,7 @@ typedef struct CDXLDemuxContext { uint8_t header[CDXL_HEADER_SIZE]; int video_stream_index; int audio_stream_index; + int64_t filesize; } CDXLDemuxContext;
static int cdxl_read_probe(AVProbeData *p) @@ -95,6 +96,8 @@ static int cdxl_read_header(AVFormatContext *s) cdxl->read_chunk = 0; cdxl->video_stream_index = -1; cdxl->audio_stream_index = -1; + + cdxl->filesize = avio_size(s->pb);
s->ctx_flags |= AVFMTCTX_NOHEADER;
@@ -108,7 +111,7 @@ static int cdxl_read_packet(AVFormatContext *s, AVPacket *pkt) uint32_t current_size, video_size, image_size; uint16_t audio_size, palette_size, width, height; int64_t pos; - int ret; + int frames, ret;
if (url_feof(pb)) return AVERROR_EOF;
@@ -129,6 +132,8 @@ static int cdxl_read_packet(AVFormatContext *s, AVPacket *pkt) audio_size = AV_RB16(&cdxl->header[22]); image_size = FFALIGN(width, 16) * height * cdxl->header[19] / 8; video_size = palette_size + image_size; + + frames = cdxl->filesize / (audio_size + video_size);
i think this could cause a division by 0 [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB DNS cache poisoning attacks, popular search engine, Google internet authority dont be evil, please
Hi,
@@ -129,6 +132,8 @@ static int cdxl_read_packet(AVFormatContext *s, AVPacket *pkt) audio_size = AV_RB16(&cdxl->header[22]); image_size = FFALIGN(width, 16) * height * cdxl->header[19] / 8; video_size = palette_size + image_size; + + frames = cdxl->filesize / (audio_size + video_size);
i think this could cause a division by 0
Sorry, fixed locally. Regards
On 12/17/12, Piotr Bandurski <ami_stuff@o2.pl> wrote:
fixes ticket #1937
Better way to set duration is via bitrate. Chunk size do not need to be constant. None of current demuxers sets duration using file size.
Hi
Better way to set duration is via bitrate. Chunk size do not need to be constant. None of current demuxers sets duration using file size.
Yes but I couldn't make it to work :( Regards
On Mon, Dec 17, 2012 at 09:06:25PM +0100, Piotr Bandurski wrote:
Hi
Better way to set duration is via bitrate. Chunk size do not need to be constant. None of current demuxers sets duration using file size.
Yes but I couldn't make it to work :(
neither could i, i also dont see a bitrate field in the header nor has anyone replied in the ticket to my question from where the bitrate could be found Thus applied as this seems to be the best that can be done -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The misfortune of the wise is better than the prosperity of the fool. -- Epicurus
participants (4)
-
Derek Buitenhuis -
Michael Niedermayer -
Paul B Mahol -
Piotr Bandurski