From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 101BA44748 for ; Tue, 25 Oct 2022 09:38:10 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id A3B3368B88B; Tue, 25 Oct 2022 12:38:07 +0300 (EEST) Received: from mail-lf1-f51.google.com (mail-lf1-f51.google.com [209.85.167.51]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 13DA868BBD4 for ; Tue, 25 Oct 2022 12:38:01 +0300 (EEST) Received: by mail-lf1-f51.google.com with SMTP id d6so20882202lfs.10 for ; Tue, 25 Oct 2022 02:38:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :from:to:cc:subject:date:message-id:reply-to; bh=VF++qst53YnNzrWzeu52Z0gsKfHU7Q1BtBrnEQwtmdE=; b=B+8fy60502GqCc8HrbfIDDN7rfv6uPTDxVlQJThcQj/ILmpDtQ+zc/GC8PUiGnRt6G t/CyzUf6KpHWCG3KNbeDtNhooRM2AMB3/Nr0/7MbiyaoCzw4vHHG4CUzd4l8MAReSxeH JlCVvigs1qlS0tqsC7yxEJKncMGLNyyuoGEBmCFoZuHTYbPhpDRwjrPjQb/zQmrMcecI 87Y4ufgCiuXv9vlJ5lgWlfVL4BdFrgMwNmV3ycE4+/F2HhXMmvKE3oZp2c1Iw3U6Ewyt ttumVzRExSSAKzv4KTH8JbPqEJi1Yd30hWz8VQmmmmsPMN39IIRxQmUE7vRxmdO1IiUK XMbQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=VF++qst53YnNzrWzeu52Z0gsKfHU7Q1BtBrnEQwtmdE=; b=qHk7KftStpUJ/akvDn1MLWWLJAGjH8THDDYs8UoVzHsTmFXKw0g1Pwd3+759Iiyvn7 TbgHvxbhEFQxnS86gpkocEidM4CZ6UkhCZMpAUWyeQwc6yq+dM8Lu0UF2bnc1620a+B0 BG1NzRaNedsgTkfH7+LpSw4k5pwbpzEgOiPynR/WpvmjwBmBaSiRuVH5f0q+GoI/fwUy MmzVT2sqQqRiH53COwJN2sKhpldv/EelFO9g3EuEZ3n0MN0CMdKUWDg98m5bl0qgRduF KY3fURw7WEtqbaTBtK/+5C2bQpszhzL5zOx2l5G3FSQJWh5Vl8Zs/NAxHxAO5vAHMJG9 tSIA== X-Gm-Message-State: ACrzQf0VPpZoFPcZRtDlYcnuKoz2TOQUS/mikiB/kGMfqEtShPbxCjDn Q9SF9WD3K8AqIQGzUpgj75nSfxqI1FVYLPYzZqpxMvbx X-Google-Smtp-Source: AMsMyM578v0U9KJXEl4riaU5E/ok+ORLbLnbXXQxBaESR58bMkqLow4+2zNuPo+B/z7PLIZty9UlpaPs8n53Eosbgf0= X-Received: by 2002:a19:f618:0:b0:4a2:4f89:163e with SMTP id x24-20020a19f618000000b004a24f89163emr12545364lfe.684.1666690679367; Tue, 25 Oct 2022 02:37:59 -0700 (PDT) MIME-Version: 1.0 References: <95e9b5f39f921151f1da2aeba099088ddb1ed0ce.1666689226.git.ffmpegagent@gmail.com> In-Reply-To: <95e9b5f39f921151f1da2aeba099088ddb1ed0ce.1666689226.git.ffmpegagent@gmail.com> From: Hendrik Leppkes Date: Tue, 25 Oct 2022 11:37:46 +0200 Message-ID: To: FFmpeg development discussions and patches Subject: Re: [FFmpeg-devel] [PATCH v9 02/25] avutil/frame: Prepare AVFrame for subtitle handling X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 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 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Tue, Oct 25, 2022 at 11:14 AM softworkz wrote: > > @@ -712,6 +712,53 @@ typedef struct AVFrame { > * Duration of the frame, in the same units as pts. 0 if unknown. > */ > int64_t duration; > + > + /** > + * Media type of the frame (audio, video, subtitles..) > + * > + * See AVMEDIA_TYPE_xxx > + */ > + enum AVMediaType type; > + > + /** > + * Number of items in the @ref subtitle_areas array. > + */ > + unsigned num_subtitle_areas; > + > + /** > + * Array of subtitle areas, may be empty. > + */ > + AVSubtitleArea **subtitle_areas; > + > + /** > + * Header containing style information for text subtitles. > + */ > + AVBufferRef *subtitle_header; > + > + /** > + * Indicates that a subtitle frame is a repeated frame for arbitrating flow > + * in a filter graph. > + * The field subtitle_timing.start_pts always indicates the original presentation > + * time, while the frame's pts field may be different. > + */ > + int repeat_sub; > + > + struct SubtitleTiming > + { > + /** > + * The display start time, in AV_TIME_BASE. > + * > + * For subtitle frames, AVFrame.pts is populated from the packet pts value, > + * which is not always the same as this value. > + */ > + int64_t start_pts; There is still no explanation here why they are not the same, why they could not just be the same, and which field a user should look at. The cover letter talks about clarity why this is needed is important, but then provides none of that. "Its required" is not an argument. So please enlighten us once again why we absolutely need two timestamps for subtitles, instead of just one. As far as I can see, subtitle frames only have one relevant time - when its supposed to be shown on the screen. What would the other time ever be good for to a user? Similarly for the duration, of course. I can even see the AVFrame.duration field in this patch snippet just above the additions that would seem to fully replace this one. - Hendrik > + > + /** > + * Display duration, in AV_TIME_BASE. > + */ > + int64_t duration; > + > + } subtitle_timing; > } AVFrame; > > > @@ -788,6 +835,8 @@ void av_frame_move_ref(AVFrame *dst, AVFrame *src); > /** > * Allocate new buffer(s) for audio or video data. > * > + * Note: For subtitle data, use av_frame_get_buffer2 > + * > * The following fields must be set on frame before calling this function: > * - format (pixel format for video, sample format for audio) > * - width and height for video > @@ -807,9 +856,39 @@ void av_frame_move_ref(AVFrame *dst, AVFrame *src); > * recommended to pass 0 here unless you know what you are doing. > * > * @return 0 on success, a negative AVERROR on error. > + * > + * @deprecated Use @ref av_frame_get_buffer2 instead and set @ref AVFrame.type > + * before calling. > */ > +attribute_deprecated > int av_frame_get_buffer(AVFrame *frame, int align); > > +/** > + * Allocate new buffer(s) for audio, video or subtitle data. > + * > + * The following fields must be set on frame before calling this function: > + * - format (pixel format for video, sample format for audio) > + * - width and height for video > + * - nb_samples and channel_layout for audio > + * - type (AVMediaType) > + * > + * This function will fill AVFrame.data and AVFrame.buf arrays and, if > + * necessary, allocate and fill AVFrame.extended_data and AVFrame.extended_buf. > + * For planar formats, one buffer will be allocated for each plane. > + * > + * @warning: if frame already has been allocated, calling this function will > + * leak memory. In addition, undefined behavior can occur in certain > + * cases. > + * > + * @param frame frame in which to store the new buffers. > + * @param align Required buffer size alignment. If equal to 0, alignment will be > + * chosen automatically for the current CPU. It is highly > + * recommended to pass 0 here unless you know what you are doing. > + * > + * @return 0 on success, a negative AVERROR on error. > + */ > +int av_frame_get_buffer2(AVFrame *frame, int align); > + > /** > * Check if the frame data is writable. > * > diff --git a/libavutil/subfmt.c b/libavutil/subfmt.c > new file mode 100644 > index 0000000000..c72ebe2a43 > --- /dev/null > +++ b/libavutil/subfmt.c > @@ -0,0 +1,45 @@ > +/* > + * Copyright (c) 2021 softworkz > + * > + * This file is part of FFmpeg. > + * > + * FFmpeg is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * FFmpeg is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with FFmpeg; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA > + */ > + > +#include > +#include "common.h" > +#include "subfmt.h" > + > +static const char sub_fmt_info[AV_SUBTITLE_FMT_NB][24] = { > + [AV_SUBTITLE_FMT_UNKNOWN] = "Unknown subtitle format", > + [AV_SUBTITLE_FMT_BITMAP] = "Graphical subtitles", > + [AV_SUBTITLE_FMT_TEXT] = "Text subtitles (plain)", > + [AV_SUBTITLE_FMT_ASS] = "Text subtitles (ass)", > +}; > + > +const char *av_get_subtitle_fmt_name(enum AVSubtitleType sub_fmt) > +{ > + if (sub_fmt < 0 || sub_fmt >= AV_SUBTITLE_FMT_NB) > + return NULL; > + return sub_fmt_info[sub_fmt]; > +} > + > +enum AVSubtitleType av_get_subtitle_fmt(const char *name) > +{ > + for (int i = 0; i < AV_SUBTITLE_FMT_NB; i++) > + if (!strcmp(sub_fmt_info[i], name)) > + return i; > + return AV_SUBTITLE_FMT_NONE; > +} > diff --git a/libavutil/subfmt.h b/libavutil/subfmt.h > index 791b45519f..3b8a0613b2 100644 > --- a/libavutil/subfmt.h > +++ b/libavutil/subfmt.h > @@ -21,6 +21,9 @@ > #ifndef AVUTIL_SUBFMT_H > #define AVUTIL_SUBFMT_H > > +#include > + > +#include "buffer.h" > #include "version.h" > > enum AVSubtitleType { > @@ -65,4 +68,48 @@ enum AVSubtitleType { > AV_SUBTITLE_FMT_NB, ///< number of subtitle formats, DO NOT USE THIS if you want to link with shared libav* because the number of formats might differ between versions. > }; > > +typedef struct AVSubtitleArea { > +#define AV_NUM_BUFFER_POINTERS 1 > + > + enum AVSubtitleType type; > + int flags; > + > + int x; ///< top left corner of area. > + int y; ///< top left corner of area. > + int w; ///< width of area. > + int h; ///< height of area. > + int nb_colors; ///< number of colors in bitmap palette (@ref pal). > + > + /** > + * Buffers and line sizes for the bitmap of this subtitle. > + * > + * @{ > + */ > + AVBufferRef *buf[AV_NUM_BUFFER_POINTERS]; > + int linesize[AV_NUM_BUFFER_POINTERS]; > + /** > + * @} > + */ > + > + uint32_t pal[256]; ///< RGBA palette for the bitmap. > + > + char *text; ///< 0-terminated plain UTF-8 text > + char *ass; ///< 0-terminated ASS/SSA compatible event line. > + > +} AVSubtitleArea; > + > +/** > + * Return the name of sub_fmt, or NULL if sub_fmt is not > + * recognized. > + */ > +const char *av_get_subtitle_fmt_name(enum AVSubtitleType sub_fmt); > + > +/** > + * Return a subtitle format corresponding to name, or AV_SUBTITLE_FMT_NONE > + * on error. > + * > + * @param name Subtitle format name. > + */ > +enum AVSubtitleType av_get_subtitle_fmt(const char *name); > + > #endif /* AVUTIL_SUBFMT_H */ > -- > ffmpeg-codebot > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".