Quoting Alexandra Hájková (2015-04-17 12:34:13)
> typedef struct ASFContext {
> - const AVClass *class;
> - int asfid2avid[128]; ///< conversion table from asf ID 2
> AVStream ID
> - ASFStream streams[128]; ///< it's max number and it's not
> that big
> - uint32_t stream_bitrates[128]; ///< max number of streams, bitrate
> for each (for streaming)
> - AVRational dar[128];
> - char stream_languages[128][6]; ///< max number of streams,
> language for each (RFC1766, e.g. en-US)
> - /* non streamed additonnal info */
> - /* packet filling */
> - int packet_size_left;
> - /* only for reading */
> - uint64_t data_offset; ///< beginning of the first data
> packet
> - uint64_t data_object_offset; ///< data object offset (excl. GUID
> & size)
> - uint64_t data_object_size; ///< size of the data object
> - int index_read;
> -
> - ASFMainHeader hdr;
> -
> - int packet_flags;
> - int packet_property;
> - int packet_timestamp;
> - int packet_segsizetype;
> - int packet_segments;
> - int packet_seq;
> - int packet_replic_size;
> - int packet_key_frame;
> - int packet_padsize;
> - unsigned int packet_frag_offset;
> - unsigned int packet_frag_size;
> - int64_t packet_frag_timestamp;
> - int packet_multi_size;
> - int packet_obj_size;
> - int packet_time_delta;
> - int packet_time_start;
> - int64_t packet_pos;
> -
> - int stream_index;
> -
> - ASFStream *asf_st; ///< currently decoded stream
> -
> - int no_resync_search;
> - int export_xmp;
> + int data_reached;
> + int is_simple_index; // is simple index present or not 1/0
> + int is_header;
> +
> + uint64_t preroll;
> + uint64_t nb_packets; // ASF packets
> + uint32_t packet_size;
> + int64_t send_time;
> + int duration;
> +
> + uint32_t b_flags; // flags with broadcast flag
> + uint32_t prop_flags; // file properties object flags
> +
> + uint64_t data_size; // data object size
> + int64_t header_obj_size;
I already asked in the last review -- any reason for this to be signed?
Also, is this variable even needed?
> +static int asf_read_value(AVFormatContext *s, uint8_t *name, uint16_t
> name_len,
> + uint16_t val_len, int type, AVDictionary **met)
> +{
> + int ret;
> + uint8_t *value;
> + uint16_t buflen = 2 * val_len + 1;
> + AVIOContext *pb = s->pb;
> +
> + value = av_malloc(buflen);
> + if (!value)
> + return AVERROR(ENOMEM);
> + if (type == ASF_UNICODE) {
> + // get_asf_string reads UTF-16 and converts it to UTF-8 which needs
> longer buffer
> + if ((ret = get_asf_string(pb, val_len, value, buflen)) < 0) {
> + av_freep(&value);
What part of 'same below' in the last review was unclear?
> - asf_st->stream_language_index = 128; // invalid stream index means no
> language info
> +static int asf_read_metadata_obj(AVFormatContext *s, const GUIDParseTable *g)
> +{
> + ASFContext *asf = s->priv_data;
> + AVIOContext *pb = s->pb;
> + uint64_t size = avio_rl64(pb);
> + uint16_t nb_recs = avio_rl16(pb); // number of records in the
> Description Records list
> + int i, ret;
>
> - if (!(asf->hdr.flags & 0x01)) { // if we aren't streaming...
> - st->duration = asf->hdr.play_time /
> - (10000000 / 1000) - start_time;
> - }
> - ff_get_guid(pb, &g);
> + for (i = 0; i < nb_recs; i++) {
> + uint16_t name_len, buflen, type, val_len, st_num;
> + uint8_t *name = NULL;
>
> - test_for_ext_stream_audio = 0;
> - if (!ff_guidcmp(&g, &ff_asf_audio_stream)) {
> - type = AVMEDIA_TYPE_AUDIO;
> - } else if (!ff_guidcmp(&g, &ff_asf_video_stream)) {
> - type = AVMEDIA_TYPE_VIDEO;
> - } else if (!ff_guidcmp(&g, &ff_asf_jfif_media)) {
> - type = AVMEDIA_TYPE_VIDEO;
> - st->codec->codec_id = AV_CODEC_ID_MJPEG;
> - } else if (!ff_guidcmp(&g, &ff_asf_command_stream)) {
> - type = AVMEDIA_TYPE_DATA;
> - } else if (!ff_guidcmp(&g, &ff_asf_ext_stream_embed_stream_header)) {
> - test_for_ext_stream_audio = 1;
> - type = AVMEDIA_TYPE_UNKNOWN;
> - } else {
> - return -1;
> - }
> - ff_get_guid(pb, &g);
> - avio_skip(pb, 8); /* total_size */
> - type_specific_size = avio_rl32(pb);
> - avio_rl32(pb);
> - st->id = avio_rl16(pb) & 0x7f; /* stream id */
> - // mapping of asf ID to AV stream ID;
> - asf->asfid2avid[st->id] = s->nb_streams - 1;
> -
> - avio_rl32(pb);
> -
> - if (test_for_ext_stream_audio) {
> - ff_get_guid(pb, &g);
> - if (!ff_guidcmp(&g, &ff_asf_ext_stream_audio_stream)) {
> - type = AVMEDIA_TYPE_AUDIO;
> - is_dvr_ms_audio = 1;
> - ff_get_guid(pb, &g);
> - avio_rl32(pb);
> - avio_rl32(pb);
> - avio_rl32(pb);
> - ff_get_guid(pb, &g);
> - avio_rl32(pb);
> - }
> + avio_skip(pb, 2); // skip reserved field
> + st_num = avio_rl16(pb);
> + name_len = avio_rl16(pb);
> + buflen = 2 * name_len + 1;
> + if (!name_len)
> + break;
> + type = avio_rl16(pb);
> + val_len = avio_rl32(pb);
> + name = av_malloc(name_len);
> + if (!name)
> + return AVERROR(ENOMEM);
> + avio_get_str16le(pb, name_len, name,
> + buflen);
> + if ((ret = process_metadata(s, name, name_len, val_len, type,
> &asf->asf_met[st_num])) < 0)
st_num is a 16-bit number, so this is a potential invalid write
> +static int asf_read_content_desc(AVFormatContext *s, const GUIDParseTable *g)
> +{
> + ASFContext *asf = s->priv_data;
> + AVIOContext *pb = s->pb;
> + int i;
> + static const char *const titles[] =
> + { "Title", "Author", "Copyright", "Description", "Rate" };
> + uint16_t len[5], buflen[5] = {0};
> + uint8_t *ch;
> + uint64_t size = avio_rl64(pb);
> +
> + for (i = 0; i < 5; i++) {
> + len[i] = avio_rl16(pb);
> + // utf8 string should be <= 2 * utf16 string, extra byte for the
> terminator
> + buflen[i] = 2 * len[i] + 1;
> + }
>
> - st->codec->codec_tag = tag1;
> - st->codec->codec_id = ff_codec_get_id(ff_codec_bmp_tags, tag1);
> - if (tag1 == MKTAG('D', 'V', 'R', ' ')) {
> - st->need_parsing = AVSTREAM_PARSE_FULL;
> - /* issue658 contains wrong w/h and MS even puts a fake seq header
> - * with wrong w/h in extradata while a correct one is in the
> stream.
> - * maximum lameness */
> - st->codec->width =
> - st->codec->height = 0;
> - av_freep(&st->codec->extradata);
> - st->codec->extradata_size = 0;
> + for (i = 0; i < 5; i++) {
> + if (buflen[i]) {
Can buflen be zero?
> +static int asf_read_properties(AVFormatContext *s, const GUIDParseTable *g)
> {
> ASFContext *asf = s->priv_data;
> AVIOContext *pb = s->pb;
> - ff_asf_guid g;
> - int ext_len, payload_ext_ct, stream_ct, i;
> - uint32_t leak_rate, stream_num;
> - unsigned int stream_languageid_index;
> -
> - avio_rl64(pb); // starttime
> - avio_rl64(pb); // endtime
> - leak_rate = avio_rl32(pb); // leak-datarate
> - avio_rl32(pb); // bucket-datasize
> - avio_rl32(pb); // init-bucket-fullness
> - avio_rl32(pb); // alt-leak-datarate
> - avio_rl32(pb); // alt-bucket-datasize
> - avio_rl32(pb); // alt-init-bucket-fullness
> - avio_rl32(pb); // max-object-size
> - avio_rl32(pb); // flags
> (reliable,seekable,no_cleanpoints?,resend-live-cleanpoints, rest of bits
> reserved)
> - stream_num = avio_rl16(pb); // stream-num
> -
> - stream_languageid_index = avio_rl16(pb); // stream-language-id-index
> - if (stream_num < 128)
> - asf->streams[stream_num].stream_language_index =
> stream_languageid_index;
> -
> - avio_rl64(pb); // avg frametime in 100ns units
> - stream_ct = avio_rl16(pb); // stream-name-count
> - payload_ext_ct = avio_rl16(pb); // payload-extension-system-count
> -
> - if (stream_num < 128)
> - asf->stream_bitrates[stream_num] = leak_rate;
> -
> - for (i = 0; i < stream_ct; i++) {
> - avio_rl16(pb);
> - ext_len = avio_rl16(pb);
> - avio_skip(pb, ext_len);
> - }
> -
> - for (i = 0; i < payload_ext_ct; i++) {
> - ff_get_guid(pb, &g);
> - avio_skip(pb, 2);
> - ext_len = avio_rl32(pb);
> - avio_skip(pb, ext_len);
> + uint64_t creation_time;
> +
> + avio_rl64(pb); // read object size
> + avio_skip(pb, 16); // skip File ID
> + avio_skip(pb, 8); // skip File size
> + creation_time = avio_rl64(pb);
> + if (!(asf->b_flags & ASF_FLAG_BROADCAST)) {
> + struct tm tmbuf;
> + struct tm *tm;
> + char buf[64] = {0};
As I already said last time, there does not seem to be any need to
initialize buf.
> +// returns data object offset when reading this object for the first time
> +static int asf_read_data(AVFormatContext *s, const GUIDParseTable *g)
> {
> - AVIOContext *pb = s->pb;
> ASFContext *asf = s->priv_data;
> - int i, count, name_len, ret;
> - char name[1024];
> + AVIOContext *pb = s->pb;
> + uint64_t size = asf->data_size = avio_rl64(pb);
> + int i;
>
> - avio_rl64(pb); // reserved 16 bytes
> - avio_rl64(pb); // ...
> - count = avio_rl32(pb); // markers count
> - avio_rl16(pb); // reserved 2 bytes
> - name_len = avio_rl16(pb); // name length
> - for (i = 0; i < name_len; i++)
> - avio_r8(pb); // skip the name
> -
> - for (i = 0; i < count; i++) {
> - int64_t pres_time;
> - int name_len;
> -
> - avio_rl64(pb); // offset, 8 bytes
> - pres_time = avio_rl64(pb); // presentation time
> - pres_time -= asf->hdr.preroll * 10000;
> - avio_rl16(pb); // entry length
> - avio_rl32(pb); // send time
> - avio_rl32(pb); // flags
> - name_len = avio_rl32(pb); // name length
> - if ((ret = avio_get_str16le(pb, name_len * 2, name,
> - sizeof(name))) < name_len)
> - avio_skip(pb, name_len - ret);
> - avpriv_new_chapter(s, i, (AVRational) { 1, 10000000 }, pres_time,
> - AV_NOPTS_VALUE, name);
> + if (!asf->data_reached && pb->seekable) {
> + asf->data_reached = 1;
> + asf->data_offset = asf->offset;
> }
>
> + if (!(asf->b_flags & ASF_FLAG_BROADCAST) && !s->streams[i]->duration) {
What is the value of i here?
> +static int asf_read_header(AVFormatContext *s)
> +{
> + ASFContext *asf = s->priv_data;
> + AVIOContext *pb = s->pb;
> + const GUIDParseTable *g = NULL;
> + ff_asf_guid guid;
> + int i, ret;
> + uint64_t size;
> +
> + asf->preroll = 0;
> + asf->is_simple_index = 0;
> + ff_get_guid(pb, &guid);
> + if (ff_guidcmp(&guid, &ff_asf_header))
> + return AVERROR_INVALIDDATA;
> + asf->header_obj_size = avio_rl64(pb);
> + avio_skip(pb, 6); // skip number of header objects and 2 reserved bytes
> + asf->data_reached = 0;
> +
> + // 1 is here instead of pb->eof_reached because, Data are skipped for
> the first time,
> + // Index object is processed and got eof and then seeking back to the
> Data is performed.
So, when exactly does the index get read?
And again, I asked this already in the last review.
> + while (1) {
> + if (avio_tell(pb) == asf->offset) {
> + if (asf->data_reached)
> + avio_seek(pb, asf->first_packet_offset, SEEK_SET);
> + break;
> + }
> + asf->offset = avio_tell(pb);
> + if ((ret = ff_get_guid(pb, &guid)) < 0) {
> + if (ret == AVERROR_EOF && asf->data_reached) {
> + avio_seek(pb, asf->first_packet_offset, SEEK_SET);
> + break;
> + } else
> + return ret;
> + }
> + g = find_guid(guid);
> + if (g) {
> + asf->unknown_offset = asf->offset;
> + asf->is_header = 1;
> + if ((ret = g->read_object(s, g)) < 0)
> + return ret;
> + } else {
> + size = avio_rl64(pb);
> + align_position(pb, asf->offset, size);
> + }
> + if (asf->data_reached && !pb->seekable)
> + break;
> }
>
> - if (!asf->index_read)
> - ret = asf_build_simple_index(s, stream_index);
> + for (i = 0; i < asf->nb_streams; i++) {
> + const char *rfc1766 = asf->langs[asf->asf_st[i]->lang_idx];
> + AVStream *st = s->streams[asf->asf_st[i]->index];
> + set_language(s, rfc1766, &st->metadata);
> + }
>
> - if (!ret && asf->index_read && st->index_entries) {
> - index = av_index_search_timestamp(st, pts, flags);
> - if (index >= 0) {
> - /* find the position */
> - pos = st->index_entries[index].pos;
> + for (i = 0; i < ASF_MAX_STREAMS; i++) {
> + AVStream *st = NULL;
>
> - /* do the seek */
> - av_log(s, AV_LOG_DEBUG, "SEEKTO: %"PRId64"\n", pos);
> - avio_seek(s->pb, pos, SEEK_SET);
> - asf_reset_header(s);
> - return 0;
> - }
> + st = find_stream(s, i);
> + if (st)
> + av_dict_copy(&st->metadata, asf->asf_met[i],
> AV_DICT_IGNORE_SUFFIX);
> }
> - /* no index or seeking by index failed */
> - if (ff_seek_frame_binary(s, stream_index, pts, flags) < 0)
> - return -1;
> - asf_reset_header(s);
> +
> return 0;
> +
Extra empty line.
--
Anton Khirnov
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
