On Sat, Feb 28, 2015 at 10:17 AM, Alexandra Hájková
<[email protected]> wrote:
> From: alexandra <alexandra@sai>
Can we have a proper email?
> The old one is result of reverse engeneering and guesswork.
> The new one had been written following the now-available specification.
>
> This work is part of Outreach Program for Women Summer 2014 activities.
You are changing FATE tests, mention here the reasons for that, and
how that impacts.
> ---
> libavformat/asf.h | 21 -
> libavformat/asfdec.c | 2508
> ++++++++++++++++++++++-------------------
> libavformat/asfenc.c | 21 +
> tests/ref/fate/lossless-wma | 2 +-
> tests/ref/fate/wmv8-drm-nodec | 1 -
> tests/ref/seek/lavf-asf | 54 +-
> 6 files changed, 1375 insertions(+), 1232 deletions(-)
>
> + // presentation time offset, equal for all streams, should be equal to
> send time, !100-nanosecond units
> + uint64_t time_offset;
> + int64_t offset; // offset of the current object
> + int64_t data_offset;
> + int64_t first_packet_offset; // packet offset
> + int64_t unknown_offset; // for top level header objects or subobjects
> whithout specified behavior
without
> @@ -247,19 +393,28 @@ static int asf_read_picture(AVFormatContext *s, int len)
> ret = AVERROR(ENOMEM);
> goto fail;
> }
> + if (!(asf->asf_st[asf->nb_streams] = av_mallocz(sizeof(*asf_st))))
nit: I wouldn't mind having this check on two lines
> + return AVERROR(ENOMEM);
> + asf_st = asf->asf_st[asf->nb_streams];
> + asf_st->pkt = NULL;
> +
> st->disposition |= AV_DISPOSITION_ATTACHED_PIC;
> - st->codec->codec_type = AVMEDIA_TYPE_VIDEO;
> + st->codec->codec_type = asf_st->type = AVMEDIA_TYPE_VIDEO;
> st->codec->codec_id = id;
> st->attached_pic = pkt;
> - st->attached_pic.stream_index = st->index;
> + st->attached_pic.stream_index = asf_st->index = st->index;
> st->attached_pic.flags |= AV_PKT_FLAG_KEY;
>
> + asf->nb_streams++;
> +
> if (*desc)
> - av_dict_set(&st->metadata, "title", desc, AV_DICT_DONT_STRDUP_VAL);
> + if (av_dict_set(&st->metadata, "title", desc,
> AV_DICT_DONT_STRDUP_VAL) < 0)
> + av_log(s, AV_LOG_WARNING, "av_dict_set failed.\n");
> else
> av_freep(&desc);
>
> - av_dict_set(&st->metadata, "comment", ff_id3v2_picture_types[type], 0);
> + if (av_dict_set(&st->metadata, "comment", ff_id3v2_picture_types[type],
> 0) < 0)
> + av_log(s, AV_LOG_WARNING, "av_dict_set failed.\n");
>
> return 0;
>}
> +
> +static int read_ext_content(AVFormatContext *s, const GUIDParseTable *g)
> {
> - char *value;
> - int64_t off = avio_tell(s->pb);
> + ASFContext *asf = s->priv_data;
> + AVIOContext *pb = s->pb;
> + uint64_t size = avio_rl64(pb);
> + uint16_t nb_desc = avio_rl16(pb);
> + int i, ret;
>
> - if ((unsigned)len >= (UINT_MAX - 1) / 2)
> - return;
> + for (i = 0; i < nb_desc; i++) {
> + uint16_t name_len, type, val_len;
> + uint8_t *name = NULL;
>
> - value = av_malloc(2 * len + 1);
> - if (!value)
> - goto finish;
> -
> - if (type == 0) { // UTF16-LE
> - avio_get_str16le(s->pb, len, value, 2 * len + 1);
> - } else if (type == 1) { // byte array
> - if (!strcmp(key, "WM/Picture")) { // handle cover art
> - asf_read_picture(s, len);
> - } else if (!strcmp(key, "ID3")) { // handle ID3 tag
> - get_id3_tag(s, len);
> - } else {
> - av_log(s, AV_LOG_VERBOSE, "Unsupported byte array in tag %s.\n",
> key);
> + name_len = avio_rl16(pb);
> + if (!name_len)
> + return AVERROR_INVALIDDATA;
Not sure how much this error is common, but maybe it'd be worth adding
a message.
> + name = av_malloc(name_len);
> + if (!name)
> + return AVERROR(ENOMEM);
> + avio_get_str16le(pb, name_len * sizeof(*name), name,
> + name_len);
> + type = avio_rl16(pb);
> + val_len = avio_rl16(pb);
> +
> + if ((ret = process_metadata(s, name, val_len, type, &s->metadata)) <
> 0)
> + return ret;
> + }
> +
> + align_position(pb, asf->offset, size);
> + return 0;
> +}
> +> -static int asf_read_file_properties(AVFormatContext *s, int64_t size)
> +static int read_content(AVFormatContext *s, const GUIDParseTable *g)
> {
> ASFContext *asf = s->priv_data;
> AVIOContext *pb = s->pb;
> -
> - ff_get_guid(pb, &asf->hdr.guid);
> - asf->hdr.file_size = avio_rl64(pb);
> - asf->hdr.create_time = avio_rl64(pb);
> - avio_rl64(pb); /* number of packets */
> - asf->hdr.play_time = avio_rl64(pb);
> - asf->hdr.send_time = avio_rl64(pb);
> - asf->hdr.preroll = avio_rl32(pb);
> - asf->hdr.ignore = avio_rl32(pb);
> - asf->hdr.flags = avio_rl32(pb);
> - asf->hdr.min_pktsize = avio_rl32(pb);
> - asf->hdr.max_pktsize = avio_rl32(pb);
> - if (asf->hdr.min_pktsize >= (1U << 29))
> - return AVERROR_INVALIDDATA;
> - asf->hdr.max_bitrate = avio_rl32(pb);
> - s->packet_size = asf->hdr.max_pktsize;
> + int i;
> + static const char *const titles[] = { "Title", "Author", "Copyright",
> + "Description", "Rate" };
nit: it's either all on one line or curly braces go on separate lines
> + uint16_t len[5], max_len = 0;
> + uint8_t *ch;
> + uint64_t size = avio_rl64(pb);
> +
> + for (i = 0; i < 5; i++) {
> + len[i] = avio_rl16(pb);
> + max_len = FFMAX(max_len, len[i]);
> + }
> + if (max_len) {
> + ch = av_malloc(max_len * sizeof(*ch));
> + if (!ch)
> + return(AVERROR(ENOMEM));
> + for (i = 0; i < 5; i++) {
> + read_metadata(s, titles[i], len[i], ch, max_len);
> + }
> + av_freep(&ch);
> + }
> + align_position(pb, asf->offset, size);
>
> return 0;
> }
>
> -static int asf_read_stream_properties(AVFormatContext *s, int64_t size)
> +static int read_properties(AVFormatContext *s, const GUIDParseTable *g)
> {
> ASFContext *asf = s->priv_data;
> AVIOContext *pb = s->pb;
> - AVStream *st;
> - ASFStream *asf_st;
> - ff_asf_guid g;
> - enum AVMediaType type;
> - int type_specific_size, sizeX;
> - unsigned int tag1;
> - int64_t pos1, pos2, start_time;
> - int test_for_ext_stream_audio, is_dvr_ms_audio = 0;
> -
> - if (s->nb_streams == ASF_MAX_STREAMS) {
> - av_log(s, AV_LOG_ERROR, "too many streams\n");
> - return AVERROR(EINVAL);
> + uint64_t nb_packets, creation_date;
> + uint32_t max_packet_size;
> + struct tm tmbuf;
> + struct tm *tm;
> + char buf[100];
> +
> + avio_rl64(pb); // read object size
> + avio_skip(pb, 16); // skip File ID
> + avio_skip(pb, 8); // skip File size
> + creation_date = avio_rl64(pb);
> + if (!(asf->b_flags & IS_BROADCAST)) {
> + // creation date is in 100 ns units from 1 Jan 1601
> + // conversion to seconds
nit: double space and first you use 'ns' then 'seconds', if you use
's' it will fit on one line
> + creation_date /= 10000000;
> + // there are 11644473600 seconds etween 1 Jan 1601 and 1 Jan 1970
> + creation_date -= 11644473600;
> + tm = gmtime_r(&creation_date, &tmbuf);
> + if (tm) {
> + if (!strftime(buf, sizeof(buf), "%Y-%m-%d %H:%M:%S", tm))
> + buf[0] = '\0';
> + } else
> + buf[0] = '\0';
> }
> + if (av_dict_set(&s->metadata, "creation date", buf, 0) < 0)
> + av_log(s, AV_LOG_WARNING, "av_dict_set failed.\n");
> + nb_packets = avio_rl64(pb);
> + asf->duration = avio_rl64(pb) / 10000; // stream duration
> + avio_skip(pb, 8); //skip send duration
nit: space after //
> + asf->preroll = avio_rl64(pb);
> + asf->b_flags = avio_rl32(pb);
> + avio_skip(pb, 4); // skip minimal packet size
> + max_packet_size = avio_rl32(pb);
> + avio_skip(pb, 4); //skip max_bitrate
nit: space after //
> + asf->nb_packets = nb_packets;
> + asf->packet_size = max_packet_size;
>
> - pos1 = avio_tell(pb);
> + return 0;
> +}
> - }
> +static const GUIDParseTable gdef[] = {
> + {"Data", {0x75, 0xB2, 0x26, 0x36, 0x66, 0x8E,
> 0x11, 0xCF, 0xA6, 0xD9, 0x00, 0xAA, 0x00, 0x62, 0xCE, 0x6C}, read_data, 1},
[...]
> + {"Payload Extension System Degradable JPEG", {0x00, 0xE1, 0xAF, 0x06,
> 0x7B, 0xEC, 0x11, 0xD1, 0xA5, 0x82, 0x00, 0xC0, 0x4F, 0xC2, 0x9C, 0xFB},
> read_unknown, 1},
> +};
nit: space after { and before }
>
> - if (padsize >= packet_length) {
> - av_log(s, AV_LOG_ERROR,
> - "invalid padsize %"PRIu32" at:%"PRId64"\n", padsize,
> avio_tell(pb));
> - return -1;
> + if (avio_tell(pb) >= (asf->packet_offset + asf->packet_size -
> asf->pad_len)) {
nit: extra parenthesis
> + asf->sub_left = 0;
> + if (!asf->nb_mult_left) {
> + avio_skip(pb, asf->pad_len);
> + if (avio_tell(pb) != asf->packet_offset + asf->packet_size) {
> + av_log(s, AV_LOG_WARNING,
> + "Position %"PRId64" wrong, should be %"PRId64"\n",
> + avio_tell(pb), asf->packet_offset + asf->packet_size);
> + avio_seek(pb, asf->packet_offset + asf->packet_size,
> SEEK_SET);
> + }
> + }
> }
> +static int do_deinterleaving(AVFormatContext *s, ASFPacket *asf_pkt, int
> st_num)
how about asf_deinterleave? After all, function always do something ;)
> +/*
> + * Find a timestamp for the requested position within the payload
> + * where pos (position) is the offset inside the Data Object.
imho 'where the pos variable (representing position in the bytestream) is...'
In general looks okish, I'd ask you to add asf_ prefixes to all the
functions, since it make it easier to debug and keep things
consistent.
--
Vittorio
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
