[vlc-commits] [Git][videolan/vlc][master] 10 commits: subpicture: use picture_CopyPixels to copy planes

Jean-Baptiste Kempf (@jbk) gitlab at videolan.org
Wed Aug 2 20:03:24 UTC 2023



Jean-Baptiste Kempf pushed to branch master at VideoLAN / VLC


Commits:
c2060bd0 by Steve Lhomme at 2023-08-02T19:45:31+00:00
subpicture: use picture_CopyPixels to copy planes

The destination picture is created with the same format as the source.
We can safely use picture_CopyPixels.

- - - - -
eaeef519 by Steve Lhomme at 2023-08-02T19:45:31+00:00
vout/display: use VLC_SUCCESS for all control errors

So they are all on the same page.

- - - - -
1f104d7e by Steve Lhomme at 2023-08-02T19:45:31+00:00
video_output: fix RGB conversion comment

We only need the conversion for the snapshot. We neither feed that picture
to the display module, nor use it for blending.

- - - - -
bc48e53e by Steve Lhomme at 2023-08-02T19:45:31+00:00
vout_subpicture: match the hardcoded chroma list size

If SPU_CHROMALIST_COUNT changes and the table doesn't fit anymore,
we will notice.

This value might also be shared with display modules so they don't write
more chromas that can actually be handled.

- - - - -
744e6c22 by Steve Lhomme at 2023-08-02T19:45:31+00:00
vlc_filter: filter_chain_AppendConverter return a VLC error code

This is already assumed in many places.

- - - - -
133be85a by Steve Lhomme at 2023-08-02T19:45:31+00:00
vout/display: use the display pool for filters

This is already what is being used. But it's better to do it explicitly rather
than pretending we may create a pool with just 3 pictures.

That code cannot be called if vout_OpenWrapper() has not been called. So the
pool size has already been determined.

- - - - -
889aa2ee by Steve Lhomme at 2023-08-02T19:45:31+00:00
vout: remove dead code for pool size test

The pool no longer comes from the vout which may have allocated less pictures.
Now we allocate exactly the amount requested. The size of display_pool
is always reserved_picture.

- - - - -
7687d530 by Steve Lhomme at 2023-08-02T19:45:31+00:00
vout_wrapper: use explicit variables for the converter private pool

At least we know what we're trying to put in there.

- - - - -
0d6eb71f by Steve Lhomme at 2023-08-02T19:45:31+00:00
vout: remove internal display_pool variable

We never get pictures from that pool.

Test for the existence of display_pool is equivalent to testing the existence
of private_pool since one can't exist without the other.

- - - - -
0a5e19e2 by Steve Lhomme at 2023-08-02T19:45:31+00:00
vout: move VOUT_MAX_PICTURES where it's used

Other parts of the code don't need to know about this.

- - - - -


13 changed files:

- include/vlc_filter.h
- modules/video_chroma/chain.c
- modules/video_chroma/cvpx.c
- modules/video_chroma/gst_mem.c
- modules/video_filter/canvas.c
- src/misc/filter_chain.c
- src/misc/subpicture.c
- src/video_output/display.c
- src/video_output/video_output.c
- src/video_output/vout_internal.h
- src/video_output/vout_private.h
- src/video_output/vout_subpictures.c
- src/video_output/vout_wrapper.c


Changes:

=====================================
include/vlc_filter.h
=====================================
@@ -537,8 +537,7 @@ VLC_API filter_t *filter_chain_AppendFilter(filter_chain_t *chain,
  *
  * \param chain filter chain to append a filter to
  * \param fmt_out filter output format
- * \retval 0 on success
- * \retval -1 on failure
+ * \retval VLC_SUCCESS on success
  */
 VLC_API int filter_chain_AppendConverter(filter_chain_t *chain,
     const es_format_t *fmt_out);


=====================================
modules/video_chroma/chain.c
=====================================
@@ -428,7 +428,7 @@ static int BuildFilterChain( filter_t *p_filter )
         if( filter_chain_AppendConverter( p_sys->p_chain,
                                           &fmt_mid ) != VLC_SUCCESS )
             continue;
-        
+
         p_sys->p_video_filter =
             filter_chain_AppendFilter( p_sys->p_chain,
                                        p_filter->psz_name, p_filter->p_cfg,
@@ -461,10 +461,12 @@ static int CreateChain( filter_t *p_filter, const es_format_t *p_fmt_mid )
     filter_sys_t *p_sys = p_filter->p_sys;
     filter_chain_Reset( p_sys->p_chain, &p_filter->fmt_in, p_filter->vctx_in, &p_filter->fmt_out );
 
-    if( filter_chain_AppendConverter( p_sys->p_chain, p_fmt_mid ) )
-        return VLC_EGENERIC;
+    int i_ret = filter_chain_AppendConverter( p_sys->p_chain, p_fmt_mid );
+    if ( i_ret != VLC_SUCCESS )
+        return i_ret;
 
-    if( filter_chain_AppendConverter( p_sys->p_chain, &p_filter->fmt_out ) )
+    i_ret = filter_chain_AppendConverter( p_sys->p_chain, &p_filter->fmt_out );
+    if ( i_ret != VLC_SUCCESS )
         goto error;
 
     p_filter->vctx_out = filter_chain_GetVideoCtxOut( p_sys->p_chain );


=====================================
modules/video_chroma/cvpx.c
=====================================
@@ -673,11 +673,11 @@ Open_chain_CVPX(filter_t *filter)
 
     /* Append intermediate CVPX chroma */
     ret = filter_chain_AppendConverter(chain, &fmt_out);
-    if (ret != 0)
+    if (ret != VLC_SUCCESS)
         goto error;
     /* Append final chroma, either CVPX or software. */
     ret = filter_chain_AppendConverter(chain, NULL);
-    if (ret != 0)
+    if (ret != VLC_SUCCESS)
         goto error;
 
     struct vlc_video_context *vctx_out =


=====================================
modules/video_chroma/gst_mem.c
=====================================
@@ -97,12 +97,12 @@ static int Open(filter_t *p_filter)
 
     int ret;
     ret = filter_chain_AppendConverter(p_chain, &fmt_intermediate);
-    if (ret != 0)
-        return VLC_EGENERIC;
+    if (ret != VLC_SUCCESS)
+        return ret;
 
     ret = filter_chain_AppendConverter(p_chain, NULL);
-    if (ret != 0)
-        return VLC_EGENERIC;
+    if (ret != VLC_SUCCESS)
+        return ret;
 
     p_filter->p_sys = p_chain;
     p_filter->ops = &chain_ops;


=====================================
modules/video_filter/canvas.c
=====================================
@@ -339,11 +339,12 @@ static int Activate( filter_t *p_filter )
 
     filter_chain_Reset( p_sys->p_chain, &p_filter->fmt_in, p_filter->vctx_in, &fmt );
     /* Append scaling module */
-    if ( filter_chain_AppendConverter( p_sys->p_chain, NULL ) )
+    int ret = filter_chain_AppendConverter( p_sys->p_chain, NULL );
+    if ( ret != VLC_SUCCESS )
     {
         msg_Err( p_filter, "Could not append scaling filter" );
         free( p_sys );
-        return VLC_EGENERIC;
+        return ret;
     }
 
     /* Append croppadd module if we actually do cropping or padding instead of just scaling*/


=====================================
src/misc/filter_chain.c
=====================================
@@ -305,7 +305,7 @@ int filter_chain_AppendConverter( filter_chain_t *chain,
     const es_format_t *fmt_out )
 {
     return filter_chain_AppendInner( chain, NULL, chain->conv_cap, NULL,
-                                     fmt_out ) != NULL ? 0 : -1;
+                                     fmt_out ) != NULL ? VLC_SUCCESS : VLC_EGENERIC;
 }
 
 void filter_chain_DeleteFilter( filter_chain_t *chain, filter_t *filter )


=====================================
src/misc/subpicture.c
=====================================
@@ -338,10 +338,6 @@ subpicture_region_t* subpicture_region_Copy( subpicture_region_t *p_region_src )
 
     p_region_dst->p_text = text_segment_Copy( p_region_src->p_text );
 
-    //Palette is already copied by subpicture_region_New, we just have to duplicate p_pixels
-    for (int i = 0; i < p_region_src->p_picture->i_planes; i++)
-        memcpy(p_region_dst->p_picture->p[i].p_pixels,
-               p_region_src->p_picture->p[i].p_pixels,
-               p_region_src->p_picture->p[i].i_lines * p_region_src->p_picture->p[i].i_pitch);
+    picture_CopyPixels(p_region_dst->p_picture, p_region_src->p_picture);
     return p_region_dst;
 }


=====================================
src/video_output/display.c
=====================================
@@ -259,10 +259,8 @@ static picture_t *VideoBufferNew(filter_t *filter)
            osys->display_fmt.i_width  == fmt->i_width  &&
            osys->display_fmt.i_height == fmt->i_height);
 
-    picture_pool_t *pool = vout_GetPool(vd, 3);
-    if (!pool)
-        return NULL;
-    return picture_pool_Get(pool);
+    assert(picture_pool_GetSize(osys->pool) >= 3);
+    return picture_pool_Get(osys->pool);
 }
 
 static vlc_decoder_device * DisplayHoldDecoderDevice(vlc_object_t *o, void *sys)
@@ -287,7 +285,7 @@ static int VoutDisplayCreateRender(vout_display_t *vd)
 
     osys->converters = filter_chain_NewVideo(vd, false, &owner);
     if (unlikely(osys->converters == NULL))
-        return -1;
+        return VLC_ENOMEM;
 
     video_format_t v_src = osys->source;
     v_src.i_sar_num = 0;
@@ -299,7 +297,7 @@ static int VoutDisplayCreateRender(vout_display_t *vd)
 
     const bool convert = memcmp(&v_src, &v_dst, sizeof(v_src)) != 0;
     if (!convert)
-        return 0;
+        return VLC_SUCCESS;
 
     msg_Dbg(vd, "A filter to adapt decoder %4.4s to display %4.4s is needed",
             (const char *)&v_src.i_chroma, (const char *)&v_dst.i_chroma);
@@ -317,7 +315,7 @@ static int VoutDisplayCreateRender(vout_display_t *vd)
     es_format_Clean(&dst);
     es_format_Clean(&src);
 
-    if (ret != 0) {
+    if (ret != VLC_SUCCESS) {
         msg_Err(vd, "Failed to adapt decoder format to display");
         filter_chain_Delete(osys->converters);
         osys->converters = NULL;
@@ -475,21 +473,23 @@ static int vout_SetSourceAspect(vout_display_t *vd,
                                 unsigned sar_num, unsigned sar_den)
 {
     vout_display_priv_t *osys = container_of(vd, vout_display_priv_t, display);
-    int ret = 0;
+    int err1, err2 = VLC_SUCCESS;
 
     if (sar_num > 0 && sar_den > 0) {
         osys->source.i_sar_num = sar_num;
         osys->source.i_sar_den = sar_den;
     }
 
-    if (vout_display_Control(vd, VOUT_DISPLAY_CHANGE_SOURCE_ASPECT))
-        ret = -1;
+    err1 = vout_display_Control(vd, VOUT_DISPLAY_CHANGE_SOURCE_ASPECT);
 
     /* If a crop ratio is requested, recompute the parameters */
-    if (osys->crop.mode != VOUT_CROP_NONE && vout_UpdateSourceCrop(vd))
-        ret = -1;
+    if (osys->crop.mode != VOUT_CROP_NONE)
+        err2 = vout_UpdateSourceCrop(vd);
 
-    return ret;
+    if (err1 != VLC_SUCCESS)
+        return err1;
+
+    return err2;
 }
 
 void VoutFixFormatAR(video_format_t *fmt)
@@ -532,7 +532,7 @@ void vout_UpdateDisplaySourceProperties(vout_display_t *vd, const video_format_t
         err2 = vout_UpdateSourceCrop(vd);
     }
 
-    if (err1 || err2)
+    if (err1 != VLC_SUCCESS || err2 != VLC_SUCCESS)
         vout_display_Reset(vd);
 }
 
@@ -542,7 +542,7 @@ void vout_display_SetSize(vout_display_t *vd, unsigned width, unsigned height)
 
     osys->cfg.display.width  = width;
     osys->cfg.display.height = height;
-    if (vout_display_Control(vd, VOUT_DISPLAY_CHANGE_DISPLAY_SIZE))
+    if (vout_display_Control(vd, VOUT_DISPLAY_CHANGE_DISPLAY_SIZE) != VLC_SUCCESS)
         vout_display_Reset(vd);
 }
 
@@ -554,7 +554,7 @@ void vout_SetDisplayFitting(vout_display_t *vd, enum vlc_video_fitting fit)
         return; /* nothing to do */
 
     osys->cfg.display.fitting = fit;
-    if (vout_display_Control(vd, VOUT_DISPLAY_CHANGE_DISPLAY_FILLED))
+    if (vout_display_Control(vd, VOUT_DISPLAY_CHANGE_DISPLAY_FILLED) != VLC_SUCCESS)
         vout_display_Reset(vd);
 }
 
@@ -571,7 +571,7 @@ void vout_SetDisplayZoom(vout_display_t *vd, unsigned num, unsigned den)
         return; /* zoom has no effects */
     if (onum * den == num * oden)
         return; /* zoom has not changed */
-    if (vout_display_Control(vd, VOUT_DISPLAY_CHANGE_ZOOM))
+    if (vout_display_Control(vd, VOUT_DISPLAY_CHANGE_ZOOM) != VLC_SUCCESS)
         vout_display_Reset(vd);
 }
 
@@ -589,7 +589,7 @@ void vout_SetDisplayAspect(vout_display_t *vd, unsigned dar_num, unsigned dar_de
         sar_den = 0;
     }
 
-    if (vout_SetSourceAspect(vd, sar_num, sar_den))
+    if (vout_SetSourceAspect(vd, sar_num, sar_den) != VLC_SUCCESS)
         vout_display_Reset(vd);
 }
 
@@ -601,7 +601,7 @@ void vout_SetDisplayCrop(vout_display_t *vd,
     if (!vout_CropEqual(crop, &osys->crop)) {
         osys->crop = *crop;
 
-        if (vout_UpdateSourceCrop(vd))
+        if (vout_UpdateSourceCrop(vd) != VLC_SUCCESS)
             vout_display_Reset(vd);
     }
 }


=====================================
src/video_output/video_output.c
=====================================
@@ -896,7 +896,7 @@ static void ChangeFilters(vout_thread_sys_t *vout)
         {
             msg_Dbg(&vout->obj, "Adding a filter to compensate for format changes");
             if (filter_chain_AppendConverter(sys->filter.chain_interactive,
-                                             &fmt_target) != 0) {
+                                             &fmt_target) != VLC_SUCCESS) {
                 msg_Err(&vout->obj, "Failed to compensate for the format changes, removing all filters");
                 DelAllFilterCallbacks(vout);
                 filter_chain_Reset(sys->filter.chain_static,      &fmt_target, vctx_target, &fmt_target);
@@ -1053,7 +1053,7 @@ static picture_t *ConvertRGB32AndBlend(vout_thread_sys_t *vout, picture_t *pic,
                        NULL /* TODO output video context of blender */,
                        &dst);
 
-    if (filter_chain_AppendConverter(filterc, &dst) != 0)
+    if (filter_chain_AppendConverter(filterc, &dst) != VLC_SUCCESS)
     {
         filter_chain_Delete(filterc);
         return NULL;
@@ -1210,7 +1210,7 @@ static int PrerenderPicture(vout_thread_sys_t *sys, picture_t *filtered,
                 {
                     /* Blending failed, likely because the picture is opaque or
                      * read-only. Try to convert the opaque picture to a
-                     * software RGB32 one before blending it. */
+                     * software RGB32 to generate a snapshot. */
                     if (do_snapshot)
                     {
                         picture_t *copy = ConvertRGB32AndBlend(sys, blent, subpic);
@@ -1664,7 +1664,6 @@ static int vout_Start(vout_thread_sys_t *vout, vlc_video_context *vctx, const vo
     vlc_mutex_unlock(&sys->window_lock);
 
     sys->decoder_fifo = picture_fifo_New();
-    sys->private.display_pool = NULL;
     sys->private.private_pool = NULL;
 
     sys->filter.configuration = NULL;
@@ -1734,7 +1733,7 @@ static int vout_Start(vout_thread_sys_t *vout, vlc_video_context *vctx, const vo
         vout_SetDisplayAspect(sys->display, num, den);
     vlc_queuedmutex_unlock(&sys->display_lock);
 
-    assert(sys->private.display_pool != NULL && sys->private.private_pool != NULL);
+    assert(sys->private.private_pool != NULL);
 
     sys->displayed.current       = NULL;
     sys->displayed.decoded       = NULL;
@@ -1829,7 +1828,7 @@ static void vout_ReleaseDisplay(vout_thread_sys_t *vout)
         filter_DeleteBlend(sys->spu_blend);
 
     /* Destroy the rendering display */
-    if (sys->private.display_pool != NULL)
+    if (sys->private.private_pool != NULL)
         vout_FlushUnlocked(vout, true, VLC_TICK_MAX);
 
     vlc_queuedmutex_lock(&sys->display_lock);
@@ -1860,7 +1859,7 @@ static void vout_ReleaseDisplay(vout_thread_sys_t *vout)
         picture_fifo_Delete(sys->decoder_fifo);
         sys->decoder_fifo = NULL;
     }
-    assert(sys->private.display_pool == NULL);
+    assert(sys->private.private_pool == NULL);
 
     vlc_mutex_lock(&sys->window_lock);
     vout_display_window_SetMouseHandler(sys->display_cfg.window, NULL, NULL);


=====================================
src/video_output/vout_internal.h
=====================================
@@ -29,14 +29,6 @@
 
 typedef struct input_thread_t input_thread_t;
 
-/* It should be high enough to absorbe jitter due to difficult picture(s)
- * to decode but not too high as memory is not that cheap.
- *
- * It can be made lower at compilation time if needed, but performance
- * may be degraded.
- */
-#define VOUT_MAX_PICTURES (20)
-
 /**
  * Vout configuration
  */


=====================================
src/video_output/vout_private.h
=====================================
@@ -40,7 +40,6 @@ struct vout_thread_private_t
     } interlacing;
 
     picture_pool_t  *private_pool;
-    picture_pool_t  *display_pool;
 };
 
 /* */


=====================================
src/video_output/vout_subpictures.c
=====================================
@@ -1946,6 +1946,10 @@ subpicture_t *spu_Render(spu_t *spu,
         VLC_CODEC_YUVP,
         0,
     };
+    static_assert(ARRAY_SIZE(chroma_list_default_yuv) <= SPU_CHROMALIST_COUNT,
+                  "YUV chroma list too large");
+    static_assert(ARRAY_SIZE(chroma_list_default_rgb) <= SPU_CHROMALIST_COUNT,
+                  "RGB chroma list too large");
 
     if (!chroma_list || *chroma_list == 0)
         chroma_list = vlc_fourcc_IsYUV(fmt_dst->i_chroma) ? chroma_list_default_yuv


=====================================
src/video_output/vout_wrapper.c
=====================================
@@ -52,6 +52,14 @@ static void VoutViewpointMoved(void *sys, const vlc_viewpoint_t *vp)
 /* Minimum number of display picture */
 #define DISPLAY_PICTURE_COUNT (1)
 
+/* It should be high enough to absorb jitter due to difficult picture(s)
+ * to decode but not too high as memory is not that cheap.
+ *
+ * It can be made lower at compilation time if needed, but performance
+ * may be degraded.
+ */
+#define VOUT_MAX_PICTURES (20)
+
 /*****************************************************************************
  *
  *****************************************************************************/
@@ -81,8 +89,6 @@ vout_display_t *vout_OpenWrapper(vout_thread_t *vout, vout_thread_private_t *sys
     if (vd == NULL)
         return NULL;
 
-    sys->display_pool = NULL;
-
     const unsigned private_picture  = 4; /* XXX 3 for filter, 1 for SPU */
     const unsigned kept_picture     = 1; /* last displayed picture */
     const unsigned reserved_picture = DISPLAY_PICTURE_COUNT +
@@ -93,26 +99,18 @@ vout_display_t *vout_OpenWrapper(vout_thread_t *vout, vout_thread_private_t *sys
     if (display_pool == NULL)
         goto error;
 
-#ifndef NDEBUG
-    if ( picture_pool_GetSize(display_pool) < reserved_picture )
-        msg_Warn(vout, "Not enough display buffers in the pool, requested %u got %u",
-                 reserved_picture, picture_pool_GetSize(display_pool));
-#endif
-
-    if (!vout_IsDisplayFiltered(vd) &&
-        picture_pool_GetSize(display_pool) >= reserved_picture) {
+    if (!vout_IsDisplayFiltered(vd)) {
         sys->private_pool = picture_pool_Reserve(display_pool, private_picture);
     } else {
         sys->private_pool =
             picture_pool_NewFromFormat(vd->source,
                                        __MAX(VOUT_MAX_PICTURES,
-                                             reserved_picture - DISPLAY_PICTURE_COUNT));
+                                             private_picture + kept_picture));
     }
     if (sys->private_pool == NULL) {
         picture_pool_Release(display_pool);
         goto error;
     }
-    sys->display_pool = display_pool;
 
 #ifdef _WIN32
     var_Create(vout, "video-wallpaper", VLC_VAR_BOOL|VLC_VAR_DOINHERIT);
@@ -132,10 +130,10 @@ error:
  *****************************************************************************/
 void vout_CloseWrapper(vout_thread_t *vout, vout_thread_private_t *sys, vout_display_t *vd)
 {
-    assert(sys->display_pool && sys->private_pool);
+    assert(sys->private_pool);
 
     picture_pool_Release(sys->private_pool);
-    sys->display_pool = NULL;
+    sys->private_pool = NULL;
 
 #ifdef _WIN32
     var_DelCallback(vout, "video-wallpaper", Forward, vd);



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/2ea41a7ca922aa2f4a796f04c681c9e7031e7673...0a5e19e2f9852f5bfa71b695d89e85e5b82d9976

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/2ea41a7ca922aa2f4a796f04c681c9e7031e7673...0a5e19e2f9852f5bfa71b695d89e85e5b82d9976
You're receiving this email because of your account on code.videolan.org.


VideoLAN code repository instance


More information about the vlc-commits mailing list