[Ffmpeg-devel] [PATCH] FLV metadata decoding
Michael Niedermayer
michaelni
Thu Nov 30 10:23:29 CET 2006
Hi
On Wed, Nov 29, 2006 at 08:08:39PM -0800, Allan Hsu wrote:
> Attached is a patch that attempts to read enough information out of
> onMetaData tags in FLV containers to create stream information so
> that av_find_stream_info() doesn't read through the entire stream,
> but only if the onMetaData tag is the first tag in the stream (and
> thus, functions like a header). I left the existing onMetaData code
> in the packet parser in place, but it probably isn't necessary now
> (and it doesn't do too much as it is). Please let me know if I need
> to make any changes for acceptance.
>
> -Allan
>
> Index: libavformat/flvdec.c
> ===================================================================
> --- libavformat/flvdec.c (revision 7031)
> +++ libavformat/flvdec.c (working copy)
> @@ -26,10 +26,376 @@
> */
> #include "avformat.h"
>
> +#define FLV_FLAG_VIDEO 1
> +#define FLV_FLAG_AUDIO 4
tabs arent allowed in svn
> +
> +#define FLV_TAG_TYPE_AUDIO 0x08
> +#define FLV_TAG_TYPE_VIDEO 0x09
> +#define FLV_TAG_TYPE_META 0x12
this should be an enum
> +
> +#define FLV_DATA_TYPE_NUMBER 0x00
> +#define FLV_DATA_TYPE_BOOL 0x01
> +#define FLV_DATA_TYPE_STRING 0x02
> +#define FLV_DATA_TYPE_OBJECT 0x03
> +#define FLV_DATA_TYPE_NULL 0x05
> +#define FLV_DATA_TYPE_UNDEFINED 0x06
> +#define FLV_DATA_TYPE_REFERENCE 0x07
> +#define FLV_DATA_TYPE_MIXEDARRAY 0x08
> +#define FLV_DATA_TYPE_ARRAY 0x0a
> +#define FLV_DATA_TYPE_DATE 0x0b
> +#define FLV_DATA_TYPE_UNSUPPORTED 0x0d
enum
> +
> +#define FLV_AUDIO_TYPE_UNCOMPRESSED 0
> +#define FLV_AUDIO_TYPE_ADPCM 1
> +#define FLV_AUDIO_TYPE_MP3 2
> +#define FLV_AUDIO_TYPE_UNCOMPRESSED_LE 3
> +#define FLV_AUDIO_TYPE_NELLYMOSER8M 5
> +#define FLV_AUDIO_TYPE_NELLYMOSER 6
enum
> +
> +#define FLV_VIDEO_TYPE_H263 2
> +#define FLV_VIDEO_TYPE_SCREEN 3
> +#define FLV_VIDEO_TYPE_VP6 4
enum
> +
> +#define FLV_DATA_END_OF_OBJECT 0x09
all the defines should be in a common header like flv.h and they should
be used by the muxer and demuxer, and this should be a seperate patch
> +
> +typedef struct {
> + double duration;
> + int videocodecid;
> + int audiocodecid;
> + double videoframerate;
> + unsigned int videowidth;
> + unsigned int videoheight;
> + unsigned int audiosamplerate;
> + unsigned int audiosamplesize;
> + unsigned int audiochannels;
> +} flv_header_info;
information should be directly read into AVStream/AVCodecContext, an
additional layer is not ok
> +
> +static int flv_meta_read_metadata(AVFormatContext *s, int flags);
> +static int flv_meta_read_mixedarray(AVFormatContext *s, flv_header_info *header_info, unsigned int pos_limit);
> +static int flv_data_get_string(ByteIOContext *ioc, char *buffer, int maxlen);
> +static int flv_data_skip_object(ByteIOContext *ioc, unsigned int *type);
useless
> +
> +static int flv_meta_read_metadata(AVFormatContext *s, int flags) {
> + flv_header_info header_info;
> + ByteIOContext *ioc = &s->pb;
> + offset_t offset;
> + unsigned int type, bodylength, next_tag;
> + char buffer[11];
the metadata parsing is duplicated, ANY code duplication causes a patch
to be rejected
[...]
> + if(header_info.audiocodecid == 0 || header_info.audiochannels == 0 || header_info.audiosamplerate == 0
> + || header_info.audiosamplesize == 0)
> + goto bail;
this stuff is unrelated to AVFMTCTX_NOHEADER IIRC, AVFMTCTX_NOHEADER is
just about the possibility of adding new streams in random packets
basically a 2 line change like
if(nb_streams==2)
flags &= ~AVFMTCTX_NOHEADER;
in read_packet() should fix the issue if there is an audio and video stream
if there is not then i dont see how parsing a large chunk of the file can
be avoded, nothing gurantees that no video or audio stream starts at a later
point or is there some rule in the spec or some no-audio-stream metadata i missed?
but besides this, metadata reading support is certainly welcome ...
[...]
> + switch(header_info.audiosamplerate) {
> + case 44000:
> + audioStream->codec->sample_rate = 44100;
> + break;
> + case 22000:
> + audioStream->codec->sample_rate = 22050;
> + break;
> + case 11000:
> + audioStream->codec->sample_rate = 11025;
> + break;
> + case 5500:
> + audioStream->codec->sample_rate = 5512;
> + break;
> + default:
> + audioStream->codec->sample_rate = header_info.audiosamplerate;
> + }
if the samplerate isnt correct then dont use it and leave it to
av_find_stream_info() to fill it in
> +
> + switch(header_info.audiocodecid) {
> + case FLV_AUDIO_TYPE_UNCOMPRESSED:
> + if(header_info.audiosamplesize == 16)
> + audioStream->codec->codec_id = CODEC_ID_PCM_S16BE;
> + else if(header_info.audiosamplesize == 8)
> + audioStream->codec->codec_id = CODEC_ID_PCM_S8;
> + else
> + audioStream->codec->codec_tag = header_info.audiocodecid;
> + break;
> +
> + case FLV_AUDIO_TYPE_ADPCM:
> + audioStream->codec->codec_id = CODEC_ID_ADPCM_SWF;
> + break;
> +
> + case FLV_AUDIO_TYPE_MP3:
> + audioStream->codec->codec_id = CODEC_ID_MP3;
> + audioStream->need_parsing = 1;
> + break;
> +
> + case FLV_AUDIO_TYPE_UNCOMPRESSED_LE:
> + if(header_info.audiosamplesize == 16)
> + audioStream->codec->codec_id = CODEC_ID_PCM_S16LE;
> + else if(header_info.audiosamplesize == 8)
> + audioStream->codec->codec_id = CODEC_ID_PCM_S8;
> + else
> + goto bail;
> + break;
> +
> + default:
> + av_log(s, AV_LOG_INFO, "Unsupported audio codec in META tag: (%x)\n", header_info.audiocodecid);
> + audioStream->codec->codec_tag = header_info.audiocodecid;
> + }
the flv->ffmpeg codec_id setting is duplicated too, so is the one for video
[...]
> + return(0);
superfluous ()
> +
> +bail:
> + //remove any streams we might have created
> + //set the stream to the location of the next tag
> + url_fseek(ioc, next_tag, SEEK_SET);
> +
> + return(-1);
> +}
> +
> +static int flv_meta_read_mixedarray(AVFormatContext *s, flv_header_info *header_info, unsigned int pos_limit) {
> + ByteIOContext *ioc = &s->pb;
> + unsigned int type, count, i;
> + int keylen;
> + char buffer[256];
> +
> + if((url_ftell(ioc) + 6) > pos_limit)
> + goto bail;
the code for bail just does a return -1 so these goto must be removed
[...]
> static int flv_probe(AVProbeData *p)
> {
> const uint8_t *d;
> -
> +
cosmetic change
[...]
> - if(!url_is_streamed(&s->pb)){
> - const int fsize= url_fsize(&s->pb);
> - url_fseek(&s->pb, fsize-4, SEEK_SET);
> - size= get_be32(&s->pb);
> - url_fseek(&s->pb, fsize-3-size, SEEK_SET);
> - if(size == get_be24(&s->pb) + 11){
> - s->duration= get_be24(&s->pb) * (int64_t)AV_TIME_BASE / 1000;
> - }
> - }
> + if(flv_meta_read_metadata(s, flags) < 0) {
> + // there was no metadata packet or not enough metadata in the packet.
> + s->ctx_flags |= AVFMTCTX_NOHEADER;
> +
> + if(!url_is_streamed(&s->pb)) {
> + const int fsize= url_fsize(&s->pb);
> + url_fseek(&s->pb, fsize-4, SEEK_SET);
> + size= get_be32(&s->pb);
> + url_fseek(&s->pb, fsize-3-size, SEEK_SET);
> + if(size == get_be24(&s->pb) + 11) {
> + s->duration= get_be24(&s->pb) * (int64_t)AV_TIME_BASE / 1000;
> + }
> + }
>
> - url_fseek(&s->pb, offset, SEEK_SET);
> + url_fseek(&s->pb, offset, SEEK_SET);
> + }
reindention of code is not allowed in patches which contain functional
changes
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is
More information about the ffmpeg-devel
mailing list