On 2015-02-16 20:24, Mark Reid wrote:
On Mon, Feb 16, 2015 at 4:07 AM, <tomas.har...@codemill.se> wrote:
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.
will do.
Another thing: since this is adds another function to the API maybe the
minor version should be bumped? I can't recall if the avio_* functions
are strictly internal. If they are then there might not be a need.
+#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
:)
I wasn't to thrilled about the 2 bytes either, but seeing that the
function is part of the public api, I didn't want to break anything.
Good call :)
+ 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?
I couldn't find too much stuff writing utf-16 strings, id3v2, mmst and
subtiles. I think most of them calculate the length after writing.
would utils/avstring.c be good place to put it?
Possibly, but I don't think we need to extend the API if nothing else
needs it.
+/*
+ * 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.
Okay, I'll split it into two patches and send a new set, thanks for
taking
the time to review
No problem
/Tomas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
