[vlc-devel] [PATCH 02/10] vout: parse crop string into dedicated structure

Steve Lhomme robux4 at ycbcr.xyz
Mon Feb 8 09:37:26 UTC 2021


On 2021-02-08 10:11, Rémi Denis-Courmont wrote:
> Hi,
> 
> I'll have to disagree on both points. Destination<-Source is the 
> standard order in C/C++ and widely used in VLC for similar constructs 
> such as the format helpers. As for the name, I don't see the 
> inconsistency at all. It uses literally the most common function naming 
> pattern in VLC.Mmmmmmmmm

Just look at the line above my comment and the line below. I have no 
preference but since we usually try to keep the code consistent locally 
(for better or worse) it would make sense to be consistent.
It's not a blocker.

> Le 8 février 2021 09:59:08 GMT+02:00, Steve Lhomme <robux4 at ycbcr.xyz> a 
> écrit :
> 
>     On 2021-02-06 17:27, remi at remlab.net wrote:
> 
>         From: Rémi Denis-Courmont <remi at remlab.net>
> 
>         This changes the parser function to output into a single structure
>         rather than half a dozen different variables.
> 
>         No functional changes.
>         ------------------------------------------------------------------------
>         src/video_output/video_output.c | 18 +++++++-------
>         src/video_output/vout_internal.h | 5 +---
>         src/video_output/vout_intf.c | 41 +++++++++++++++-----------------
>         3 files changed, 28 insertions(+), 36 deletions(-)
> 
>         diff --git a/src/video_output/video_output.c
>         b/src/video_output/video_output.c
>         index d01bf880d6..9ca6dab765 100644
>         --- a/src/video_output/video_output.c
>         +++ b/src/video_output/video_output.c
>         @@ -2214,24 +2214,22 @@ static void
>         vout_InitSource(vout_thread_sys_t *vout)
> 
>         char *psz_crop = var_InheritString(&vout->obj, "crop");
>         if (psz_crop) {
>         - unsigned num, den;
>         - unsigned y, x;
>         - unsigned width, height;
>         - enum vout_crop_mode mode;
>         + struct vout_crop crop;
> 
>         - if (GetCropMode(psz_crop, &mode, &num, &den,
>         - &x, &y, &width, &height))
>         + if (vout_ParseCrop(&crop, psz_crop))
>         {
>         - switch (mode)
>         + switch (crop.mode)
>         {
>         case VOUT_CROP_RATIO:
>         - vout_SetCropRatio(vout, num, den);
>         + vout_SetCropRatio(vout, crop.ratio.num, crop.ratio.den);
>         break;
>         case VOUT_CROP_WINDOW:
>         - vout_SetCropWindow(vout, x, y, width, height);
>         + vout_SetCropWindow(vout, crop.window.x, crop.window.y,
>         + crop.window.width, crop.window.height);
>         break;
>         case VOUT_CROP_BORDER:
>         - vout_SetCropBorder(vout, x, y, width, height);
>         + vout_SetCropBorder(vout, crop.border.left, crop.border.top,
>         + crop.border.right, crop.border.bottom);
>         break;
>         case VOUT_CROP_NONE:
>         break;
>         diff --git a/src/video_output/vout_internal.h
>         b/src/video_output/vout_internal.h
>         index a351106224..42e5748d54 100644
>         --- a/src/video_output/vout_internal.h
>         +++ b/src/video_output/vout_internal.h
>         @@ -126,10 +126,7 @@ struct vout_crop {
>         };
>         };
> 
>         -bool GetCropMode(const char *crop_str, enum vout_crop_mode *mode,
>         - unsigned *num, unsigned *den,
>         - unsigned *x, unsigned *y,
>         - unsigned *width, unsigned *height );
>         +bool vout_ParseCrop(struct vout_crop *, const char *crop_str);
> 
> 
>     The naming and ordering of parameters is inconsistent with the code around.
> 
>         bool GetAspectRatio(const char *ar_str, unsigned *num, unsigned
>         *den);
> 
>         /* TODO to move them to vlc_vout.h */
>         diff --git a/src/video_output/vout_intf.c
>         b/src/video_output/vout_intf.c
>         index a4ea6bf5af..daa1d70eab 100644
>         --- a/src/video_output/vout_intf.c
>         +++ b/src/video_output/vout_intf.c
>         @@ -449,22 +449,21 @@ exit:
>         free( psz_path );
>         }
> 
>         -bool GetCropMode(const char *crop_str, enum vout_crop_mode *mode,
>         - unsigned *num, unsigned *den,
>         - unsigned *x, unsigned *y,
>         - unsigned *width, unsigned *height )
>         +bool vout_ParseCrop(struct vout_crop *restrict cfg, const char
>         *crop_str)
>         {
>         - if (sscanf(crop_str, "%u:%u", num, den) == 2) {
>         - *mode = VOUT_CROP_RATIO;
>         + if (sscanf(crop_str, "%u:%u", &cfg->ratio.num,
>         &cfg->ratio.den) == 2) {
>         + cfg->mode = VOUT_CROP_RATIO;
>         } else if (sscanf(crop_str, "%ux%u+%u+%u",
>         - width, height, x, y) == 4) {
>         - *mode = VOUT_CROP_WINDOW;
>         + &cfg->window.width, &cfg->window.height,
>         + &cfg->window.x, &cfg->window.y) == 4) {
>         + cfg->mode = VOUT_CROP_WINDOW;
>         } else if (sscanf(crop_str, "%u+%u+%u+%u",
>         - x, y, width, height) == 4) {
>         - *mode = VOUT_CROP_BORDER;
>         + &cfg->border.left, &cfg->border.top,
>         + &cfg->border.right, &cfg->border.bottom) == 4) {
>         + cfg->mode = VOUT_CROP_BORDER;
>         } else if (*crop_str == '\0') {
>         - *mode = VOUT_CROP_RATIO;
>         - *num = *den = 0;
>         + cfg->mode = VOUT_CROP_RATIO;
>         + cfg->ratio.num = cfg->ratio.den = 0;
>         } else {
>         return false;
>         }
>         @@ -479,23 +478,21 @@ static int CropCallback( vlc_object_t
>         *object, char const *cmd,
>         {
>         vout_thread_t *vout = (vout_thread_t *)object;
>         VLC_UNUSED(cmd); VLC_UNUSED(oldval); VLC_UNUSED(data);
>         - unsigned num, den;
>         - unsigned y, x;
>         - unsigned width, height;
>         - enum vout_crop_mode mode;
>         + struct vout_crop crop;
> 
>         - if (GetCropMode(newval.psz_string, &mode, &num, &den,
>         - &x, &y, &width, &height)) {
>         - switch (mode)
>         + if (vout_ParseCrop(&crop, newval.psz_string)) {
>         + switch (crop.mode)
>         {
>         case VOUT_CROP_RATIO:
>         - vout_ChangeCropRatio(vout, num, den);
>         + vout_ChangeCropRatio(vout, crop.ratio.num, crop.ratio.den);
>         break;
>         case VOUT_CROP_WINDOW:
>         - vout_ChangeCropWindow(vout, x, y, width, height);
>         + vout_ChangeCropWindow(vout, crop.window.x, crop.window.y,
>         + crop.window.width, crop.window.height);
>         break;
>         case VOUT_CROP_BORDER:
>         - vout_ChangeCropBorder(vout, x, y, width, height);
>         + vout_ChangeCropBorder(vout, crop.border.left, crop.border.top,
>         + crop.border.right, crop.border.bottom);
>         break;
>         case VOUT_CROP_NONE:
>         break;
>         -- 
>         2.30.0
>         ------------------------------------------------------------------------
>         vlc-devel mailing list
>         To unsubscribe or modify your subscription options:
>         https://mailman.videolan.org/listinfo/vlc-devel
>         <https://mailman.videolan.org/listinfo/vlc-devel>
> 
>     ------------------------------------------------------------------------
>     vlc-devel mailing list
>     To unsubscribe or modify your subscription options:
>     https://mailman.videolan.org/listinfo/vlc-devel  <https://mailman.videolan.org/listinfo/vlc-devel>
> 
> 
> -- 
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser 
> ma brièveté.
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
> 


More information about the vlc-devel mailing list