Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Stefano Sabatini <stefasab@gmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH] lavu: add AVVideoHint API
Date: Wed, 2 Aug 2023 07:36:43 +0200
Message-ID: <20230802053643.GG10927@mariano> (raw)
In-Reply-To: <20230728074437.GC10927@mariano>

On date Friday 2023-07-28 09:44:37 +0200, Stefano Sabatini wrote:
> On date Wednesday 2023-07-26 10:52:57 +0000, Carotti, Elias wrote:
> > On Mon, 2023-07-24 at 01:27 +0200, Stefano Sabatini wrote:
> > > CAUTION: This email originated from outside of the organization. Do
> > > not click links or open attachments unless you can confirm the sender
> > > and know the content is safe.
> > > 
> > > 
> > <snip>
> > 
> > > > 
> > > > sorry to nitpick, but if we have this information in the single
> > > > rect
> > > > should not we use that instead?
> > > > 
> > > > Basically I see two options:
> > > > 1. generic VideoHint, storing single hint type and rects
> > > > 
> > > > for example we might have type=changed, and the rects storing only
> > > > the
> > > > changed rects
> > > > 
> > > > If we want to extend this then we need to add a new type. The
> > > > problem
> > > > might be if we want to store more than one VideoHint in the side
> > > > data
> > > > (it it possible to store more than one AVVideoHint, each one with a
> > > > different type, as side data?). Suppose for example we want to add
> > > > a
> > > > new hint type, e.g. ROI, then we can add a new AVHideoHint with
> > > > type=roi - but I don't know if we can have several frame hint side
> > > > data (each one with different individual types).
> > > > 
> > > > 2. generic VideoHint, storing rects each one containing the hint
> > > > type
> > > > 
> > > > for example we might have a list of rects, each one with
> > > > type=changed|constant|roi. This would allow one to store different
> > > > kind of hints in the same AVVideoHint structure, but at the same
> > > > time
> > > > would enable inconsistent data (for example we might have changed
> > > > and
> > > > unchanged rects in the same list of rects)
> > > > 
> > 
> 
> > I don't think it allowed such inconsistency the way it was made.
> > Basically, the semantics was that the Hint type could be either
> > CONSTANT or CHANGED and that was the basis to interpret the rects with
> > type DAMAGE. 
> 
> Yes, but then there is no guarantee that you'll have all rects with
> the same subtype, and you need to iterate and check each one to
> check the subtype (so you might have mixed constant/changed and
> unclear behavior to apply in this case).
> 
> > > > In each case, storing the type in the generic AVVideHint and in
> > > > each
> > > > rect might lead to inconsistent states (e.g. you might have generic
> > > > type=damage, and have type=roi and type=changed in the rects), in
> > > > this
> > > > case having the type in both the general structure and in each
> > > > rects
> > > > seems redundant and leads to possible data inconsistencies.
> > > > 
> > > > ...
> > > > 
> > > 
> > > > I would rather go to solution 1, but I'm not sure if having more
> > > > than
> > > > one hint in the side data (with different subtypes) is possible and
> > > > wanted. If this is the case, probably the best solution would be to
> > > > store more than one AVVideoHint (each one with an individual type
> > > > and
> > > > list of rects) in the single side data object.
> > > 
> > > Elaborating on this.
> > > 
> > > 1. Ideally we might want to extend the av_frame_get_side_data API to
> > > be able to store more than one entry per side data, by adding a
> > > unique
> > > label to the side data, but this requires probably more effort and
> > > extends the scope even further.
> > > 
> > > 2. Alternatively, we might extend the hint API like this:
> > > 
> > > AVVideoHint *av_video_hint_create_side_data(AVFrame *frame,
> > >                                             size_t nb_rects);
> > > 
> > > AVVideoHint *av_video_hint_extend_side_data(AVFrame *frame,
> > >                                             size_t nb_rects);
> > > 
> > > by marking the continuation on the previous hint (but this would
> > > complicate deserialization as you need then to iterate to get all the
> > > side data).
> > > 
> > > Or we could make a variadic function like this:
> > > AVVideoHint *av_video_hint_create_side_data(AVFrame *frame, size_t
> > > nb_rects, ...);
> > > 
> > > Alternatively, we might store in AVVideoHint more than one hint, but
> > > this somehow conflict with the general design.
> > > 
> > > ...
> > > 
> > > As a low effort approach, I would keep the initial design of a single
> > > hint type per side-data (dropping the possibility of having more than
> > > one hint type in the same side data), which should be fixed
> > > generically by implementing 1.
> > 
> > Agreed.
> > For clarity I am attaching again the patch with Anton's changes just
> > rebased on the current master branch.
> > Anton, is this still ok to be merged?
> 
> I'm fine with the current patch, the behavior can then be extended
> by extending side data with multiple-label for the same type.
> 
> Anton, if you are fine with it I would merge this one (and maybe get
> included in the next release if we are not too late already).

I'll push this in a few days unless there are more comments.
_______________________________________________
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".

  reply	other threads:[~2023-08-02  5:36 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-19 10:19 [FFmpeg-devel] [PATCH] Optimization: support for libx264's mb_info Carotti, Elias
2023-05-21 23:17 ` Stefano Sabatini
2023-05-22  9:19   ` Carotti, Elias
2023-05-29 17:56     ` Carotti, Elias
2023-06-04 15:29       ` Stefano Sabatini
2023-06-05 15:32         ` Carotti, Elias
2023-06-11 17:15           ` Stefano Sabatini
2023-06-12  8:23             ` Kieran Kunhya
2023-06-12 17:38               ` Carotti, Elias
2023-06-18 10:04                 ` Stefano Sabatini
2023-06-12 17:28             ` Carotti, Elias
2023-06-18 10:18               ` Stefano Sabatini
2023-06-21 15:53                 ` Carotti, Elias
2023-06-22  8:44                   ` Anton Khirnov
2023-06-22 17:23                     ` Carotti, Elias
2023-06-24 11:01                       ` Anton Khirnov
2023-06-26  9:50                         ` Carotti, Elias
2023-06-28 12:55                           ` Anton Khirnov
2023-06-30 17:40                             ` Carotti, Elias
2023-07-01  8:33                               ` Anton Khirnov
2023-07-03 15:51                                 ` Carotti, Elias
2023-07-07 16:27                                   ` Carotti, Elias
2023-07-09 11:05                                     ` [FFmpeg-devel] [PATCH] lavu: add AVVideoHint API Anton Khirnov
2023-07-09 13:10                                       ` Lynne
2023-07-10  8:13                                         ` Carotti, Elias
2023-07-10 12:51                                           ` Carotti, Elias
2023-07-17 22:19                                             ` Stefano Sabatini
2023-07-23 23:27                                               ` Stefano Sabatini
2023-07-26 10:52                                                 ` Carotti, Elias
2023-07-28  7:44                                                   ` Stefano Sabatini
2023-08-02  5:36                                                     ` Stefano Sabatini [this message]
2023-08-08  8:16                                                       ` Stefano Sabatini
2023-07-10  8:09                                       ` Carotti, Elias

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=20230802053643.GG10927@mariano \
    --to=stefasab@gmail.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