On 2015-02-13 01:36, Mark Reid wrote:
 /**
+ * Convert an UTF-8 string to UTF-16BE and write it.
+ * @return number of bytes written.
+ */
+int avio_put_str16be(AVIOContext *s, const char *str);

You could maybe split this patch up by making the part that adds this function a separate patch. Not that I'm super concerned.

+#define PUT_STR16(type, write) \
+    int avio_put_str16 ##type(AVIOContext *s, const char *str)\
+{\
+    const uint8_t *q = str;\
+    int ret = 0;\
+    int err = 0;\
+    while (*q) {\
+        uint32_t ch;\
+        uint16_t tmp;\
+        GET_UTF8(ch, *q++, goto invalid;)\
+        PUT_UTF16(ch, tmp, write(s, tmp); ret += 2;)\
+        continue;\
+invalid:\
+        av_log(s, AV_LOG_ERROR, "Invaid UTF8 sequence in
avio_put_str16" #type "\n");\
+        err = AVERROR(EINVAL);\
+    }\
+    write(s, 0);\

From the last e-mail:

The tests need to be updated because avio_put_str16be writes zero terminated strings and
the muxer previously wasn't.

I was going to object to this on the grounds that it wastes a whopping two bytes per UTF-16 local tag, but I suspect the possible savings are eaten up by KAG alignment anyway. Code simplicity is favorable in this case I think :)

+    if (err)\
+    return err;\
+    ret += 2;\
+    return ret;\
+}\
+
+PUT_STR16(le, avio_wl16)
+PUT_STR16(be, avio_wb16)
+
+#undef PUT_STR16

 int ff_get_v_length(uint64_t val)
 {
diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index 17ad132..c25f5fd 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -624,14 +624,44 @@ static void mxf_write_preface(AVFormatContext *s)
 }

 /*
- * Write a local tag containing an ascii string as utf-16
+ * Returns the length of the UTF-16 string, in 16-bit characters,
that would result
+ * from decoding the utf-8 string.
+ */
+static uint64_t mxf_utf16len(const char *utf8_str)
+{
+    const uint8_t *q = utf8_str;
+    uint64_t size = 0;
+    while (*q) {
+        uint32_t ch;
+        GET_UTF8(ch, *q++, goto invalid;)
+        if (ch < 0x10000)
+            size++;
+        else
+            size += 2;
+        continue;
+invalid:
+ av_log(NULL, AV_LOG_ERROR, "Invaid UTF8 sequence in mxf_utf16len\n\n");
+    }
+    size += 1;
+    return size;
+}

Maybe this could be useful elsewhere too? What's the state of UTF-16 usage in lavf?

+/*
+ * Write a local tag containing an utf-8 string as utf-16
  */
 static void mxf_write_local_tag_utf16(AVIOContext *pb, int tag, const
char *value)
 {
-    int i, size = strlen(value);
+    int size = mxf_utf16len(value);
     mxf_write_local_tag(pb, size*2, tag);
-    for (i = 0; i < size; i++)
-        avio_wb16(pb, value[i]);
+    avio_put_str16be(pb, value);
 }

Wow, this thing was really broken before.

Overall the patch looks fine, apart from maybe splitting it up.

/Tomas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to