From patchwork Fri Oct 16 03:17:57 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 22992 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id A2D4E44B321 for ; Fri, 16 Oct 2020 06:18:35 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8673E68BA3C; Fri, 16 Oct 2020 06:18:35 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id A812568BA21 for ; Fri, 16 Oct 2020 06:18:28 +0300 (EEST) Received: by mail-wm1-f65.google.com with SMTP id e2so1310169wme.1 for ; Thu, 15 Oct 2020 20:18:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=SnnQnQT1iOOpfGQM1k5RmyHdnKYPJl1psKLUsY+Up2o=; b=CLQ7HPIDJXfeLwZipMEbaz3lHtJvleNWp/A3EqOIXX4ZDPtT8COiXEMMgio4R3sjc1 o1wfT9AFN2B1SvBXGyC11FcMCPp4JLucgSYkzw7RLZOL+hYFGqt7Obrk2YfRM13NVyN1 0SdAfFSGfx4yavnig4v17/NQZKaOTiDmpKYDTQYd7I46D5A/ZY7OWmW2M4/5omCb2smX 9Gvv0rW8oQUPq0be5yn4CQAl+PYRhk1f8v7CMDzwMKPE6/c0QevfpzxLS5jpWeukEVI6 m8jC5Mgt1375GjYJgF0fCLWeu0PEh3yU7CD0COW5OsO3zBp6UTcVLZjy6j+mIt/CGgbC SQyw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=SnnQnQT1iOOpfGQM1k5RmyHdnKYPJl1psKLUsY+Up2o=; b=V/GurnYnK3oiK7hOmugSL95QUAVLtDZAyzcVd/7sXw6Dm9oFOAKxkRPdxRbvp3AHIV 2YtAdSbCogbPY+DJ1KFxCrN47+w+YnIaIcySkUJW4+kAEBhXXZW2h7CqmD50qbdcOlV4 wOY9LZwC0SEzipcj8PVg+WrJPLTINCcrzqQj+pZ9pFYUVcNa/2Jyhk8SqjJrr6V2eSb9 83HfSN0DNliKEuofiMSo7iPUj6D8hIo4GgjqFlR5Fajy3fTdwo0aO3qYk/AqUTWvfsRo fbmWV88Kc4dBBqTG6CBh4DhDgAUtVLCH1zDCY5gQ2n4H9auhCAfc0bhhOb0SfG4bMt1m 8gyg== X-Gm-Message-State: AOAM533KUSZPXxFY3Zi+GOWYXl5+ZR0pR8ZoQdWljO7cCpeS+2MM4KdH 3ShhYbnIxxH750LebdW3rP8IYNX2iAY= X-Google-Smtp-Source: ABdhPJxIgUzzd7RttUm7IJ4PSulq3YF6PdtnNxoHi3ZXWaV0fVt6Y6IaOx8vypsndjkmS6rC0G0f4g== X-Received: by 2002:a7b:ce14:: with SMTP id m20mr1504751wmc.20.1602818307515; Thu, 15 Oct 2020 20:18:27 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1aa4b.dynamic.kabel-deutschland.de. [188.193.170.75]) by smtp.gmail.com with ESMTPSA id y5sm1515830wrw.52.2020.10.15.20.18.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 15 Oct 2020 20:18:26 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Fri, 16 Oct 2020 05:17:57 +0200 Message-Id: <20201016031758.214795-2-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20201016031758.214795-1-andreas.rheinhardt@gmail.com> References: <20201016031758.214795-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 2/3] avcodec/movtextenc: Simplify writing to AVBPrint X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" The mov_text encoder uses an AVBPrint to assemble the subtitles; yet mov_text subtitles are not pure text; they also have a binary portion that was mostly handled as follows: uint32_t size = /* calculation */; size = AV_RB32(&size); av_bprint_append_data(bprint, (const char*)&size, 4); Here AV_RB32() is a no-op on big-endian systems and a LE-BE swap on little-endian systems, making the output endian-independent. Yet this is ugly and unclean: On LE systems, the variable size from the snippet above won't contain the correct value any more. Furthermore, using this pattern leads to lots of small writes to the AVBPrint. This commit therefore changes this to using a temporary buffer instead: uint8_t buf[4]; AV_WB32(buf, /* size calculation */); av_bprint_append_data(bprint, buf, 4); This method also allows to use bigger buffers holding more than one element, saving calls to av_bprint_append_data() and reducing codesize. Signed-off-by: Andreas Rheinhardt --- libavcodec/movtextenc.c | 154 +++++++++++++++++----------------------- 1 file changed, 67 insertions(+), 87 deletions(-) diff --git a/libavcodec/movtextenc.c b/libavcodec/movtextenc.c index 11db240ab7..3d0328d332 100644 --- a/libavcodec/movtextenc.c +++ b/libavcodec/movtextenc.c @@ -29,6 +29,7 @@ #include "libavutil/common.h" #include "ass_split.h" #include "ass.h" +#include "bytestream.h" #define STYLE_FLAG_BOLD (1<<0) #define STYLE_FLAG_ITALIC (1<<1) @@ -111,33 +112,28 @@ static void mov_text_cleanup(MovTextContext *s) static void encode_styl(MovTextContext *s, uint32_t tsmb_type) { int j; - uint32_t tsmb_size; - uint16_t style_entries; + 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); + uint8_t buf[12], *p = buf; + + bytestream_put_be32(&p, s->count * STYLE_RECORD_SIZE + SIZE_ADD); + bytestream_put_be32(&p, tsmb_type); + bytestream_put_be16(&p, s->count); /*The above three attributes are hard coded for now but will come from ASS style in the future*/ - av_bprint_append_any(&s->buffer, &tsmb_size, 4); - av_bprint_append_any(&s->buffer, &tsmb_type, 4); - av_bprint_append_any(&s->buffer, &style_entries, 2); + av_bprint_append_any(&s->buffer, buf, 10); for (j = 0; j < s->count; j++) { - uint16_t style_start, style_end, style_fontID; - uint32_t style_color; - - style_start = AV_RB16(&s->style_attributes[j]->style_start); - style_end = AV_RB16(&s->style_attributes[j]->style_end); - style_color = AV_RB32(&s->style_attributes[j]->style_color); - style_fontID = AV_RB16(&s->style_attributes[j]->style_fontID); - - av_bprint_append_any(&s->buffer, &style_start, 2); - av_bprint_append_any(&s->buffer, &style_end, 2); - av_bprint_append_any(&s->buffer, &style_fontID, 2); - av_bprint_append_any(&s->buffer, &s->style_attributes[j]->style_flag, 1); - av_bprint_append_any(&s->buffer, &s->style_attributes[j]->style_fontsize, 1); - av_bprint_append_any(&s->buffer, &style_color, 4); + const StyleBox *style = s->style_attributes[j]; + + p = buf; + bytestream_put_be16(&p, style->style_start); + bytestream_put_be16(&p, style->style_end); + bytestream_put_be16(&p, style->style_fontID); + bytestream_put_byte(&p, style->style_flag); + bytestream_put_byte(&p, style->style_fontsize); + bytestream_put_be32(&p, style->style_color); + + av_bprint_append_any(&s->buffer, buf, 12); } } mov_text_cleanup(s); @@ -145,32 +141,28 @@ static void encode_styl(MovTextContext *s, uint32_t tsmb_type) static void encode_hlit(MovTextContext *s, uint32_t tsmb_type) { - uint32_t tsmb_size; - uint16_t start, end; 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); - av_bprint_append_any(&s->buffer, &tsmb_type, 4); - av_bprint_append_any(&s->buffer, &start, 2); - av_bprint_append_any(&s->buffer, &end, 2); + uint8_t buf[12], *p = buf; + + bytestream_put_be32(&p, 12); + bytestream_put_be32(&p, tsmb_type); + bytestream_put_be16(&p, s->hlit.start); + bytestream_put_be16(&p, s->hlit.end); + + av_bprint_append_any(&s->buffer, buf, 12); } } static void encode_hclr(MovTextContext *s, uint32_t tsmb_type) { - uint32_t tsmb_size, color; 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); - av_bprint_append_any(&s->buffer, &color, 4); + uint8_t buf[12], *p = buf; + + bytestream_put_be32(&p, 12); + bytestream_put_be32(&p, tsmb_type); + bytestream_put_be32(&p, s->hclr.color); + + av_bprint_append_any(&s->buffer, buf, 12); } } @@ -205,25 +197,21 @@ static int encode_sample_description(AVCodecContext *avctx) ASS *ass; ASSStyle *style; int i, j; - uint32_t tsmb_size, tsmb_type, back_color = 0, style_color; - uint16_t style_start, style_end, fontID, count; + uint32_t back_color = 0; int font_names_total_len = 0; MovTextContext *s = avctx->priv_data; + uint8_t buf[30], *p = buf; - static const uint8_t display_and_justification[] = { - 0x00, 0x00, 0x00, 0x00, // uint32_t displayFlags - 0x01, // int8_t horizontal-justification - 0xFF, // int8_t vertical-justification - }; + // 0x00, 0x00, 0x00, 0x00, // uint32_t displayFlags + // 0x01, // int8_t horizontal-justification + // 0xFF, // int8_t vertical-justification // 0x00, 0x00, 0x00, 0x00, // uint8_t background-color-rgba[4] - static const uint8_t box_record[] = { // BoxRecord { - 0x00, 0x00, // int16_t top - 0x00, 0x00, // int16_t left - 0x00, 0x00, // int16_t bottom - 0x00, 0x00, // int16_t right + // 0x00, 0x00, // int16_t top + // 0x00, 0x00, // int16_t left + // 0x00, 0x00, // int16_t bottom + // 0x00, 0x00, // int16_t right // }; - }; // StyleRecord { // 0x00, 0x00, // uint16_t startChar // 0x00, 0x00, // uint16_t endChar @@ -271,25 +259,19 @@ static int encode_sample_description(AVCodecContext *avctx) (255 - ((uint32_t)style->back_color >> 24)); } - av_bprint_append_any(&s->buffer, display_and_justification, - sizeof(display_and_justification)); - back_color = AV_RB32(&back_color); - av_bprint_append_any(&s->buffer, &back_color, 4); - // BoxRecord { - av_bprint_append_any(&s->buffer, box_record, sizeof(box_record)); - // }; + bytestream_put_be32(&p, 0); // displayFlags + bytestream_put_be16(&p, 0x01FF); // horizontal/vertical justification (2x int8_t) + bytestream_put_be32(&p, back_color); + bytestream_put_be64(&p, 0); // BoxRecord - 4xint16_t: top, left, bottom, right // StyleRecord { - style_start = AV_RB16(&s->d.style_start); - style_end = AV_RB16(&s->d.style_end); - fontID = AV_RB16(&s->d.style_fontID); - style_color = AV_RB32(&s->d.style_color); - av_bprint_append_any(&s->buffer, &style_start, 2); - av_bprint_append_any(&s->buffer, &style_end, 2); - av_bprint_append_any(&s->buffer, &fontID, 2); - av_bprint_append_any(&s->buffer, &s->d.style_flag, 1); - av_bprint_append_any(&s->buffer, &s->d.style_fontsize, 1); - av_bprint_append_any(&s->buffer, &style_color, 4); + bytestream_put_be16(&p, s->d.style_start); + bytestream_put_be16(&p, s->d.style_end); + bytestream_put_be16(&p, s->d.style_fontID); + bytestream_put_byte(&p, s->d.style_flag); + bytestream_put_byte(&p, s->d.style_fontsize); + bytestream_put_be32(&p, s->d.style_color); // }; + av_bprint_append_any(&s->buffer, buf, 30); // Build font table // We can't build a complete font table since that would require @@ -317,23 +299,21 @@ static int encode_sample_description(AVCodecContext *avctx) av_dynarray_add(&s->fonts, &s->font_count, (char*)"Serif"); // FontTableBox { - tsmb_size = SIZE_ADD + 3 * s->font_count + font_names_total_len; - tsmb_size = AV_RB32(&tsmb_size); - 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); + p = buf; + bytestream_put_be32(&p, SIZE_ADD + 3 * s->font_count + font_names_total_len); // Size + bytestream_put_be32(&p, MKBETAG('f','t','a','b')); + bytestream_put_be16(&p, s->font_count); + + av_bprint_append_any(&s->buffer, buf, 10); // FontRecord { for (i = 0; i < s->font_count; i++) { - uint8_t len; - fontID = i + 1; - fontID = AV_RB16(&fontID); - av_bprint_append_any(&s->buffer, &fontID, 2); - len = strlen(s->fonts[i]); - av_bprint_append_any(&s->buffer, &len, 1); + size_t len = strlen(s->fonts[i]); + + p = buf; + bytestream_put_be16(&p, i + 1); //fontID + bytestream_put_byte(&p, len); + + av_bprint_append_any(&s->buffer, buf, 3); av_bprint_append_any(&s->buffer, s->fonts[i], len); } // };