[vlc-devel] [PATCH] kate decoder: track and use basic formatting

[ogg.k.ogg.k at googlemail.com]

>  I strongly disagree with that:
>  - you can have multiple vout.

Oh, I didn't realize.

>  - you are not assured to be called after the vout is created.
> and we tryed to remove all vlc_object_find FIND_ANYWHERE.

Fair enough. I spent a lot of time trying to work out how to get the
size of the video, and eventually that's the only way I found. I'd
be happy to change it to another method if there is one. That way
to get the size was found in the CMML plugin.

>  If kate decoding needs the video size, it should be stored in its
> metadata...

This is not possible: positions and sizes may be expressed in
relative units to the displayed video (eg, in percentages). This is
to allow placement, eg, in corners. For this, the video size must
be known at runtime. If the video size is known beforehand and
cannot change, then sizes may be expressed in pixels instead.

>  You cannot access *internal* vout fields without a risk of race
> conditions...

OK, I'll have a look to see what locking I need to add (unless your
"internal" wording means I should not access them in the first place ?).

>  Using asprintf would be simpler and way easier to review (and to avoid
> misallocating memory). If you don't want to use asprintf, at least add a
> comment stating if the '\0' is included in len0.

asprintf is a GNU extension, is it OK to use here ?

>> +                char *copy = (char*)malloc(ev->len0);
>> +                memcpy(copy, ev->text, len0);
>  strdup or strndup would be better.

It would only copy one zero - granted, that's enough as only UTF-8 is
supported at the moment, but that's the point of len0.

Thanks for the feedback.