[Ffmpeg-devel] [PATCH] apple caff demuxer
Michael Niedermayer
michaelni
Wed Apr 4 12:27:03 CEST 2007
Hi
quick review below (ill leave the final review/approval for caff to
baptiste ...)
On Wed, Apr 04, 2007 at 02:28:56AM -0400, Justin Ruggles wrote:
[...]
> >
> >>+
> >>+void ff_caff_get_codec_id(CaffContext *ctx)
> >>+{
> >>+ ctx->codec_id = CODEC_ID_NONE;
> >>+
> >>+ /* codec selection for lpcm is chosen using format description and flags */
> >>+ if(ctx->format_id == MKBETAG('l','p','c','m')) {
> >>+ /* unpacked 24-bit is not currently supported */
> >>+ if((ctx->bits_per_channel == 24) &&
> >>+ (ctx->bytes_per_packet == (ctx->channels_per_frame * 4))) {
> >>+ return;
> >>+ }
> >>+ /* floating-point lpcm is not currently supported */
> >>+ if(ctx->format_flags & CAFF_LPCM_FLAGS_IS_FLOAT) {
> >>+ return;
> >>+ }
> >>+ if(ctx->bits_per_channel == 8) {
> >>+ ctx->codec_id = CODEC_ID_PCM_S8;
> >>+ } else if(ctx->bits_per_channel == 16) {
> >>+ if(ctx->format_flags & CAFF_LPCM_FLAGS_IS_LITTLEENDIAN) {
> >>+ ctx->codec_id = CODEC_ID_PCM_S16LE;
> >>+ } else {
> >>+ ctx->codec_id = CODEC_ID_PCM_S16BE;
> >>+ }
> >>+ } else if(ctx->bits_per_channel == 24) {
> >>+ if(ctx->format_flags & CAFF_LPCM_FLAGS_IS_LITTLEENDIAN) {
> >>+ ctx->codec_id = CODEC_ID_PCM_S24LE;
> >>+ } else {
> >>+ ctx->codec_id = CODEC_ID_PCM_S24BE;
> >>+ }
> >>+ } else if(ctx->bits_per_channel == 32) {
> >>+ if(ctx->format_flags & CAFF_LPCM_FLAGS_IS_LITTLEENDIAN) {
> >>+ ctx->codec_id = CODEC_ID_PCM_S32LE;
> >>+ } else {
> >>+ ctx->codec_id = CODEC_ID_PCM_S32BE;
> >>+ }
> >
> >
> > that mess begins to be duplicated in every demuxer, maybe
> > CODEC_ID_RAWAUDIO should be added, how to handle the BE/LE case ?
>
> maybe CODEC_ID_PCM_BE / CODEC_ID_PCM_LE. and actually use the
> SampleFormat in AVCodecContext, which would make it easy to implement
> floating-point PCM decoding.
dunno, why split BE/LE ?
[...]
> + int64_t frame_cnt; ///< frame counter. used during demuxing.
this seems unneeded?
[...]
> +extern const AVCodecTag codec_caff_tags[];
ff_ prefix missing
> +
> +void ff_caff_get_codec_id(CaffContext *ctx);
> Index: libavformat/isom.c
> ===================================================================
> --- libavformat/isom.c (revision 8622)
> +++ libavformat/isom.c (working copy)
> @@ -245,3 +245,68 @@
> strncpy(to, mov_mdhd_language_map[code], 4);
> return 1;
> }
> +
> +int ff_mov_mp4_read_descr_len(ByteIOContext *pb)
> +{
> + int len = 0;
> + int count = 4;
> + while (count--) {
> + int c = get_byte(pb);
> + len = (len << 7) | (c & 0x7f);
> + if (!(c & 0x80))
> + break;
> + }
> + return len;
> +}
> +
> +static int mov_mp4_read_descr(AVFormatContext *fc, int *tag)
> +{
> + ByteIOContext *pb = &fc->pb;
> + int len;
> + *tag = get_byte(pb);
> + len = ff_mov_mp4_read_descr_len(pb);
> + dprintf(fc, "MPEG4 description: tag=0x%02x len=%d\n", *tag, len);
> + return len;
> +}
> +
> +int ff_mov_read_esds(AVFormatContext *fc, MOV_esds_t *esds)
> +{
> + AVStream *st = fc->streams[fc->nb_streams-1];
> + ByteIOContext *pb = &fc->pb;
> + int tag, len;
> +
> + /* Well, broken but suffisant for some MP4 streams */
> + get_be32(pb); /* version + flags */
> + len = mov_mp4_read_descr(fc, &tag);
> + if (tag == MP4ESDescrTag) {
> + get_be16(pb); /* ID */
> + get_byte(pb); /* priority */
> + } else
> + get_be16(pb); /* ID */
> +
> + len = mov_mp4_read_descr(fc, &tag);
> + if (tag == MP4DecConfigDescrTag) {
> + esds->object_type_id = get_byte(pb);
> + esds->stream_type = get_byte(pb);
> + esds->buffer_size_db = get_be24(pb);
> + esds->max_bitrate = get_be32(pb);
> + esds->avg_bitrate = get_be32(pb);
> +
> + st->codec->codec_id= codec_get_id(ff_mp4_obj_type, esds->object_type_id);
> + dprintf(fc, "esds object type id %d\n", esds->object_type_id);
> + len = mov_mp4_read_descr(fc, &tag);
> + if (tag == MP4DecSpecificDescrTag) {
> + dprintf(fc, "Specific MPEG4 header len=%d\n", len);
> + st->codec->extradata = av_mallocz(len + FF_INPUT_BUFFER_PADDING_SIZE);
> + if (st->codec->extradata) {
> + get_buffer(pb, st->codec->extradata, len);
> + st->codec->extradata_size = len;
> + /* from mplayer */
> + if ((*st->codec->extradata >> 3) == 29) {
> + st->codec->codec_id = CODEC_ID_MP3ON4;
> + }
> + }
> + }
> + }
> + return 0;
> +}
moving things out of mov.c and adding ff_ prefix to existing stuff should
be in a patch seperate from caff
[...]
> + for(i=0; i<ctx->num_packets; i++) {
> + if(ctx->bytes_per_packet == 0) {
> + ctx->packet_table[i].bytes = ff_mov_mp4_read_descr_len(pb);
> + } else {
> + ctx->packet_table[i].bytes = ctx->bytes_per_packet;
> + }
ctx->packet_table[i].bytes = ctx->bytes_per_packet;
if(ctx->packet_table[i].bytes == 0)
ctx->packet_table[i].bytes = ff_mov_mp4_read_descr_len(pb);
not sure if its better though, just a random suggestion
[...]
> + /* free chunk */
> + case MKBETAG('f','r','e','e'):
> + if(size < 0)
> + return AVERROR_INVALIDDATA;
> + url_fskip(pb, size);
> + break;
> +
> + default:
> + av_log(s, AV_LOG_WARNING, "skipping CAFF chunk: %08X\n", tag);
> + if(size < 0)
> + return AVERROR_INVALIDDATA;
> + url_fskip(pb, size);
default:
av_log(s, AV_LOG_WARNING, "skipping CAFF chunk: %08X\n", tag);
case MKBETAG('f','r','e','e'):
if(size < 0)
return AVERROR_INVALIDDATA;
url_fskip(pb, size);
[...]
> + /* position the stream at the start of data */
> + if(ctx->data_size >= 0 && !url_is_streamed(pb))
> + url_fseek(pb, ctx->data_start, SEEK_SET);
you dont need to check for url_is_streamed(pb) here i think ...
if it cant seek, it cant anyway ...
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070404/dc44dd8b/attachment.pgp>
More information about the ffmpeg-devel
mailing list