Re: [libav-devel] [PATCH 2/3] asfdec: read the full Metadata Object, not just aspect ratio information

2013-02-06 Thread Anton Khirnov

On Wed,  6 Feb 2013 14:35:26 +0100, Vladimir Pantelic  
wrote:
> Use the same get_tag()/get_value() as for the Extended Content Description
> but handle the 16 bit vs 32 bit difference for type 2 (BOOL)
> 
> Signed-off-by: Vladimir Pantelic 
> ---
>  libavformat/asfdec.c |   51 ++---
>  1 files changed, 27 insertions(+), 24 deletions(-)
> 
> diff --git a/libavformat/asfdec.c b/libavformat/asfdec.c
> index 1fcf5c3..e85ee8f 100644
> --- a/libavformat/asfdec.c
> +++ b/libavformat/asfdec.c
> @@ -165,11 +165,13 @@ static int asf_probe(AVProbeData *pd)
>  return 0;
>  }
>  
> -static int get_value(AVIOContext *pb, int type)
> +/* size of type 2 (BOOL) is 32bit for "Extended Content Description Object"
> + * but 16 bit for "Metadata Object" and "Metadata Library Object" */
> +static int get_value(AVIOContext *pb, int type, int type2_size)

Can't say I like this very much, but I don't see a prettier solution either, so
ok with me.

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/3] asfdec: read the full Metadata Object, not just aspect ratio information

2013-02-06 Thread Vladimir Pantelic

On 02/06/2013 02:47 PM, Luca Barbato wrote:

On 06/02/13 14:35, Vladimir Pantelic wrote:

Use the same get_tag()/get_value() as for the Extended Content Description
but handle the 16 bit vs 32 bit difference for type 2 (BOOL)


Not sure if using a different value_type and proper macro would look
cleaner:

case ASF_BOOL_ECDO:
case ASF_BOOL:
case ASF_BLAH

if it is just that one might.


not sure what you mean. value_types are as given in the ASF file.




Signed-off-by: Vladimir Pantelic
---
  libavformat/asfdec.c |   51 ++---
  1 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/libavformat/asfdec.c b/libavformat/asfdec.c
index 1fcf5c3..e85ee8f 100644
--- a/libavformat/asfdec.c
+++ b/libavformat/asfdec.c
@@ -165,11 +165,13 @@ static int asf_probe(AVProbeData *pd)
  return 0;
  }

-static int get_value(AVIOContext *pb, int type)
+/* size of type 2 (BOOL) is 32bit for "Extended Content Description Object"
+ * but 16 bit for "Metadata Object" and "Metadata Library Object" */
+static int get_value(AVIOContext *pb, int type, int type2_size)
  {
  switch (type) {
  case 2:
-return avio_rl32(pb);
+return (type2_size == 32) ? avio_rl32(pb) : avio_rl16(pb);
  case 3:
  return avio_rl32(pb);
  case 4:
@@ -274,7 +276,7 @@ fail:
  return ret;
  }

-static void get_tag(AVFormatContext *s, const char *key, int type, int len)
+static void get_tag(AVFormatContext *s, const char *key, int type, int len, 
int type2_size)
  {
  char *value;
  int64_t off = avio_tell(s->pb);
@@ -289,7 +291,7 @@ static void get_tag(AVFormatContext *s, const char *key, 
int type, int len)
  if (type == 0) { // UTF16-LE
  avio_get_str16le(s->pb, len, value, 2 * len + 1);
  } else if (type>  1&&  type<= 5) {  // boolean or DWORD or QWORD or WORD
-uint64_t num = get_value(s->pb, type);
+uint64_t num = get_value(s->pb, type, type2_size);
  snprintf(value, len, "%"PRIu64, num);
  } else if (type == 1&&  !strcmp(key, "WM/Picture")) { // handle cover art
  asf_read_picture(s, len);
@@ -555,10 +557,10 @@ static int asf_read_content_desc(AVFormatContext *s, 
int64_t size)
  len3 = avio_rl16(pb);
  len4 = avio_rl16(pb);
  len5 = avio_rl16(pb);
-get_tag(s, "title", 0, len1);
-get_tag(s, "author", 0, len2);
-get_tag(s, "copyright", 0, len3);
-get_tag(s, "comment", 0, len4);
+get_tag(s, "title", 0, len1, 32);
+get_tag(s, "author", 0, len2, 32);
+get_tag(s, "copyright", 0, len3, 32);
+get_tag(s, "comment", 0, len4, 32);
  avio_skip(pb, len5);

  return 0;
@@ -588,11 +590,11 @@ static int asf_read_ext_content_desc(AVFormatContext *s, 
int64_t size)
   * ASF stream count starts at 1. I am using 0 to the container value
   * since it's unused. */
  if (!strcmp(name, "AspectRatioX"))
-asf->dar[0].num = get_value(s->pb, value_type);
+asf->dar[0].num = get_value(s->pb, value_type, 32);
  else if (!strcmp(name, "AspectRatioY"))
-asf->dar[0].den = get_value(s->pb, value_type);
+asf->dar[0].den = get_value(s->pb, value_type, 32);
  else
-get_tag(s, name, value_type, value_len);
+get_tag(s, name, value_type, value_len, 32);
  }

  return 0;
@@ -622,13 +624,13 @@ static int asf_read_metadata(AVFormatContext *s, int64_t 
size)
  {
  AVIOContext *pb = s->pb;
  ASFContext *asf = s->priv_data;
-int n, stream_num, name_len, value_len, value_num;
+int n, stream_num, name_len, value_len;
  int ret, i;
  n = avio_rl16(pb);

  for (i = 0; i<  n; i++) {
  char name[1024];
-int av_unused value_type;
+int value_type;

  avio_rl16(pb);  // lang_list_index
  stream_num = avio_rl16(pb);
@@ -638,18 +640,19 @@ static int asf_read_metadata(AVFormatContext *s, int64_t 
size)

  if ((ret = avio_get_str16le(pb, name_len, name, sizeof(name)))<  
name_len)
  avio_skip(pb, name_len - ret);
-av_dlog(s, "%d %d %d %d %d<%s>\n",
+av_dlog(s, "%d stream %d name_len %2d type %d len %4d<%s>\n",
  i, stream_num, name_len, value_type, value_len, name);
-/* We should use get_value() here but it does not work 2 is le16
- * here but le32 elsewhere. */
-value_num = avio_rl16(pb);
-avio_skip(pb, value_len - 2);
-
-if (stream_num<  128) {
-if (!strcmp(name, "AspectRatioX"))
-asf->dar[stream_num].num = value_num;
-else if (!strcmp(name, "AspectRatioY"))
-asf->dar[stream_num].den = value_num;
+
+if (!strcmp(name, "AspectRatioX")){
+int aspect_x = get_value(s->pb, value_type, 16);
+if(stream_num<  128)
+asf->dar[stream_num].num = aspect_x;
+} else if(!strcmp(name, "AspectRatioY")){
+int as

Re: [libav-devel] [PATCH 2/3] asfdec: read the full Metadata Object, not just aspect ratio information

2013-02-06 Thread Luca Barbato
On 06/02/13 14:35, Vladimir Pantelic wrote:
> Use the same get_tag()/get_value() as for the Extended Content Description
> but handle the 16 bit vs 32 bit difference for type 2 (BOOL)

Not sure if using a different value_type and proper macro would look
cleaner:

case ASF_BOOL_ECDO:
case ASF_BOOL:
case ASF_BLAH

if it is just that one might.

> Signed-off-by: Vladimir Pantelic 
> ---
>  libavformat/asfdec.c |   51 ++---
>  1 files changed, 27 insertions(+), 24 deletions(-)
> 
> diff --git a/libavformat/asfdec.c b/libavformat/asfdec.c
> index 1fcf5c3..e85ee8f 100644
> --- a/libavformat/asfdec.c
> +++ b/libavformat/asfdec.c
> @@ -165,11 +165,13 @@ static int asf_probe(AVProbeData *pd)
>  return 0;
>  }
>  
> -static int get_value(AVIOContext *pb, int type)
> +/* size of type 2 (BOOL) is 32bit for "Extended Content Description Object"
> + * but 16 bit for "Metadata Object" and "Metadata Library Object" */
> +static int get_value(AVIOContext *pb, int type, int type2_size)
>  {
>  switch (type) {
>  case 2:
> -return avio_rl32(pb);
> +return (type2_size == 32) ? avio_rl32(pb) : avio_rl16(pb);
>  case 3:
>  return avio_rl32(pb);
>  case 4:
> @@ -274,7 +276,7 @@ fail:
>  return ret;
>  }
>  
> -static void get_tag(AVFormatContext *s, const char *key, int type, int len)
> +static void get_tag(AVFormatContext *s, const char *key, int type, int len, 
> int type2_size)
>  {
>  char *value;
>  int64_t off = avio_tell(s->pb);
> @@ -289,7 +291,7 @@ static void get_tag(AVFormatContext *s, const char *key, 
> int type, int len)
>  if (type == 0) { // UTF16-LE
>  avio_get_str16le(s->pb, len, value, 2 * len + 1);
>  } else if (type > 1 && type <= 5) {  // boolean or DWORD or QWORD or WORD
> -uint64_t num = get_value(s->pb, type);
> +uint64_t num = get_value(s->pb, type, type2_size);
>  snprintf(value, len, "%"PRIu64, num);
>  } else if (type == 1 && !strcmp(key, "WM/Picture")) { // handle cover art
>  asf_read_picture(s, len);
> @@ -555,10 +557,10 @@ static int asf_read_content_desc(AVFormatContext *s, 
> int64_t size)
>  len3 = avio_rl16(pb);
>  len4 = avio_rl16(pb);
>  len5 = avio_rl16(pb);
> -get_tag(s, "title", 0, len1);
> -get_tag(s, "author", 0, len2);
> -get_tag(s, "copyright", 0, len3);
> -get_tag(s, "comment", 0, len4);
> +get_tag(s, "title", 0, len1, 32);
> +get_tag(s, "author", 0, len2, 32);
> +get_tag(s, "copyright", 0, len3, 32);
> +get_tag(s, "comment", 0, len4, 32);
>  avio_skip(pb, len5);
>  
>  return 0;
> @@ -588,11 +590,11 @@ static int asf_read_ext_content_desc(AVFormatContext 
> *s, int64_t size)
>   * ASF stream count starts at 1. I am using 0 to the container value
>   * since it's unused. */
>  if (!strcmp(name, "AspectRatioX"))
> -asf->dar[0].num = get_value(s->pb, value_type);
> +asf->dar[0].num = get_value(s->pb, value_type, 32);
>  else if (!strcmp(name, "AspectRatioY"))
> -asf->dar[0].den = get_value(s->pb, value_type);
> +asf->dar[0].den = get_value(s->pb, value_type, 32);
>  else
> -get_tag(s, name, value_type, value_len);
> +get_tag(s, name, value_type, value_len, 32);
>  }
>  
>  return 0;
> @@ -622,13 +624,13 @@ static int asf_read_metadata(AVFormatContext *s, 
> int64_t size)
>  {
>  AVIOContext *pb = s->pb;
>  ASFContext *asf = s->priv_data;
> -int n, stream_num, name_len, value_len, value_num;
> +int n, stream_num, name_len, value_len;
>  int ret, i;
>  n = avio_rl16(pb);
>  
>  for (i = 0; i < n; i++) {
>  char name[1024];
> -int av_unused value_type;
> +int value_type;
>  
>  avio_rl16(pb);  // lang_list_index
>  stream_num = avio_rl16(pb);
> @@ -638,18 +640,19 @@ static int asf_read_metadata(AVFormatContext *s, 
> int64_t size)
>  
>  if ((ret = avio_get_str16le(pb, name_len, name, sizeof(name))) < 
> name_len)
>  avio_skip(pb, name_len - ret);
> -av_dlog(s, "%d %d %d %d %d <%s>\n",
> +av_dlog(s, "%d stream %d name_len %2d type %d len %4d <%s>\n",
>  i, stream_num, name_len, value_type, value_len, name);
> -/* We should use get_value() here but it does not work 2 is le16
> - * here but le32 elsewhere. */
> -value_num = avio_rl16(pb);
> -avio_skip(pb, value_len - 2);
> -
> -if (stream_num < 128) {
> -if (!strcmp(name, "AspectRatioX"))
> -asf->dar[stream_num].num = value_num;
> -else if (!strcmp(name, "AspectRatioY"))
> -asf->dar[stream_num].den = value_num;
> +
> +if (!strcmp(name, "AspectRatioX")){
> +int aspect_x = get_value(s->pb, value_type, 16);
> +if(stream_num < 128)
> +asf->dar[stream_num]