Andriy Gelman:
> On Thu, 15. Oct 16:06, Andreas Rheinhardt wrote:
>> Andriy Gelman:
>>> From: Andriy Gelman <andriy.gel...@gmail.com>
>>>
>>> Fixes fate-binsub-movtextenc on PPC64
>>>
>>> Currently tags are written in reverse order on BE arches. This is fixed
>>> by using MKBETAG() and AV_RB32() to be arch agnostics.
>>>
>>> Also s->font_count is of type int. On BE arches with 32bit int,
>>> count = AV_RB16(&s->font_count) will read two most significant bytes
>>> instead of the least signifcant bytes. This is fixed by assiging
>>> s->font_count to count first.
>>>
>>> The final change is modyfying the type of len. On BE arches
>>> the most signifanct byte of the int was written instead of the least
>>> significant byte.
>>>
>>> Signed-off-by: Andriy Gelman <andriy.gel...@gmail.com>
>>> ---
>>> libavcodec/movtextenc.c | 17 +++++++++++------
>>> 1 file changed, 11 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/libavcodec/movtextenc.c b/libavcodec/movtextenc.c
>>> index b2368b641b..f38cd9cba2 100644
>>> --- a/libavcodec/movtextenc.c
>>> +++ b/libavcodec/movtextenc.c
>>> @@ -116,6 +116,7 @@ static void encode_styl(MovTextContext *s, uint32_t
>>> tsmb_type)
>>> if ((s->box_flags & STYL_BOX) && s->count) {
>>> tsmb_size = s->count * STYLE_RECORD_SIZE + SIZE_ADD;
>>> tsmb_size = AV_RB32(&tsmb_size);
>>> + tsmb_type = AV_RB32(&tsmb_type);
>>> style_entries = AV_RB16(&s->count);
>>> /*The above three attributes are hard coded for now
>>> but will come from ASS style in the future*/
>>> @@ -149,6 +150,7 @@ static void encode_hlit(MovTextContext *s, uint32_t
>>> tsmb_type)
>>> if (s->box_flags & HLIT_BOX) {
>>> tsmb_size = 12;
>>> tsmb_size = AV_RB32(&tsmb_size);
>>> + tsmb_type = AV_RB32(&tsmb_type);
>>> start = AV_RB16(&s->hlit.start);
>>> end = AV_RB16(&s->hlit.end);
>>> av_bprint_append_any(&s->buffer, &tsmb_size, 4);
>>> @@ -164,6 +166,7 @@ static void encode_hclr(MovTextContext *s, uint32_t
>>> tsmb_type)
>>> if (s->box_flags & HCLR_BOX) {
>>> tsmb_size = 12;
>>> tsmb_size = AV_RB32(&tsmb_size);
>>> + tsmb_type = AV_RB32(&tsmb_type);
>>> color = AV_RB32(&s->hclr.color);
>>> av_bprint_append_any(&s->buffer, &tsmb_size, 4);
>>> av_bprint_append_any(&s->buffer, &tsmb_type, 4);
>>> @@ -172,9 +175,9 @@ static void encode_hclr(MovTextContext *s, uint32_t
>>> tsmb_type)
>>> }
>>>
>>> static const Box box_types[] = {
>>> - { MKTAG('s','t','y','l'), encode_styl },
>>> - { MKTAG('h','l','i','t'), encode_hlit },
>>> - { MKTAG('h','c','l','r'), encode_hclr },
>>> + { MKBETAG('s','t','y','l'), encode_styl },
>>> + { MKBETAG('h','l','i','t'), encode_hlit },
>>> + { MKBETAG('h','c','l','r'), encode_hclr },
>>> };
>>>
>>> const static size_t box_count = FF_ARRAY_ELEMS(box_types);
>>> @@ -316,14 +319,16 @@ static int encode_sample_description(AVCodecContext
>>> *avctx)
>>> // FontTableBox {
>>> tsmb_size = SIZE_ADD + 3 * s->font_count + font_names_total_len;
>>> tsmb_size = AV_RB32(&tsmb_size);
>>> - tsmb_type = MKTAG('f','t','a','b');
>>> - count = AV_RB16(&s->font_count);
>>> + tsmb_type = MKBETAG('f','t','a','b');
>>> + tsmb_type = AV_RB32(&tsmb_type);
>>> + count = s->font_count;
>>> + count = AV_RB16(&count);
>>> av_bprint_append_any(&s->buffer, &tsmb_size, 4);
>>> av_bprint_append_any(&s->buffer, &tsmb_type, 4);
>>> av_bprint_append_any(&s->buffer, &count, 2);
>>> // FontRecord {
>>> for (i = 0; i < s->font_count; i++) {
>>> - int len;
>>> + uint8_t len;
>>> fontID = i + 1;
>>> fontID = AV_RB16(&fontID);
>>> av_bprint_append_any(&s->buffer, &fontID, 2);
>>>
>
>> I have sent an alternative version that avoids this horrible way of
>> writing output independent of endianness by instead using small buffers
>> and writing the values into it via AV_WBx(). This also allows to combine
>> several av_bprint_append_any() calls into one. Said patch is patch #2 of
>> this patchset:
>> https://ffmpeg.org/pipermail/ffmpeg-devel/2020-October/271057.html (The
>> second email has not yet come through due to the mailing list's random
>> delay.)
>
> My patch is a smaller diff and is consistent with the style of the code.
>> Up to maintainer but I'd suggest your changes are a separate patch.
>
This encoder has no maintainer listed, so I suggest you apply your patch
to fix the endianness issue and I rebase my patches on top of yours.
Btw: Your commit message has typos: signifcant, assiging, modyfying,
signifanct.
- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
