Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Zhao Zhili <quinkblack@foxmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v2] avutil: deprecate AVRational field inside AVOption::default_val
Date: Wed, 3 May 2023 11:08:55 +0800
Message-ID: <tencent_B19E7DF2ACC63B51C355B6D8682E0315690A@qq.com> (raw)
In-Reply-To: <d420ba2a-3346-680c-3364-ddf096b1244a@gmail.com>


> 在 2023年5月2日,19:39,James Almer <jamrial@gmail.com> 写道:
> 
> On 5/2/2023 8:34 AM, Tomas Härdin wrote:
>> tis 2023-05-02 klockan 15:48 +0800 skrev Zhao Zhili:
>>> From: Zhao Zhili <zhilizhao@tencent.com>
>>> 
>>> Signed-off-by: Zhao Zhili <zhilizhao@tencent.com>
>>> ---
>>>  libavutil/opt.h     | 2 ++
>>>  libavutil/version.h | 1 +
>>>  2 files changed, 3 insertions(+)
>>> 
>>> diff --git a/libavutil/opt.h b/libavutil/opt.h
>>> index 461b5d3b6b..46915754ea 100644
>>> --- a/libavutil/opt.h
>>> +++ b/libavutil/opt.h
>>> @@ -271,8 +271,10 @@ typedef struct AVOption {
>>>          int64_t i64;
>>>          double dbl;
>>>          const char *str;
>>> +#if FF_API_AVOPTION_AVRATIONAL
>>>          /* TODO those are unused now */
>>>          AVRational q;
>>> +#endif
>> Surely rationals options are useful?
> 
> They are, but this union is where the default value is stored when you define an AVOption. At some point it seems it was decided that rational (and video_rate) type AVOptions should set dbl instead of q, which is then av_q2d()'d into an AVRational.
> 
> I'd still not remove it, as Paul said. It's harmless being there and in the future it could be used.

The field itself is harmless, but it makes big confusing. It takes me some efforts to figure out a rational type must use dbl to hold default value.

For backward compatibility, I don’t think it’s easy to switch to AVRational from dbl.

If we decide to keep it, I can add a comment to reduce the confusion.

> 
>> /Tomas
>> _______________________________________________
>> 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 sub

_______________________________________________
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".

  parent reply	other threads:[~2023-05-03  3:09 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-02  7:48 Zhao Zhili
2023-05-02  8:32 ` Paul B Mahol
2023-05-02 11:34 ` Tomas Härdin
2023-05-02 11:39   ` James Almer
2023-05-02 11:51     ` Paul B Mahol
2023-05-03  3:08     ` Zhao Zhili [this message]
2023-05-09 10:02     ` Tomas Härdin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=tencent_B19E7DF2ACC63B51C355B6D8682E0315690A@qq.com \
    --to=quinkblack@foxmail.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git