[vlc-commits] [Git][videolan/vlc][master] 10 commits: subpicture: don't preemptively call video_format_IsSimilar to update SPU's

Steve Lhomme (@robUx4) gitlab at videolan.org
Fri Nov 17 19:22:03 UTC 2023



Steve Lhomme pushed to branch master at VideoLAN / VLC


Commits:
27be18da by Steve Lhomme at 2023-11-17T18:37:42+00:00
subpicture: don't preemptively call video_format_IsSimilar to update SPU's

The callee may not care at all and we're doing many tests for nothing.

- - - - -
0c09774c by Steve Lhomme at 2023-11-17T18:37:42+00:00
libass: do region format copy once

And do use a copy with fields we never use.

- - - - -
46c21571 by Steve Lhomme at 2023-11-17T18:37:42+00:00
libass: only check the visible size changed on the source

We don't use any other fields from the source.

- - - - -
67b41e69 by Steve Lhomme at 2023-11-17T18:37:42+00:00
substext: only check the visible height on the source

We don't use any other fields from the source. No need to redo the regions
if other things changed in the source.

- - - - -
821dd57b by Steve Lhomme at 2023-11-17T18:37:42+00:00
aribcaption: only check the visible size on the source

We don't use any other fields from the source. No need to redo the regions
if other things changed in the source.

- - - - -
1e0baf46 by Steve Lhomme at 2023-11-17T18:37:42+00:00
aribcaption: don't keep the render area dimensions

We only use the value in SubpictureUpdate() and we know the value
regardless if the src/dst visible dimensions changed or not.

- - - - -
df10484b by Steve Lhomme at 2023-11-17T18:37:42+00:00
aribcaption: do region format copy once

- - - - -
dd115137 by Steve Lhomme at 2023-11-17T18:37:42+00:00
aribcaption: don't se the original picture dimensions if there's no image

The subpicture won't have any region.

- - - - -
fa3ed1e5 by Steve Lhomme at 2023-11-17T18:37:42+00:00
arib/susbtext: don't check the output format for changes

We never use the output format. We only need to create the regions
once.

- - - - -
6928d765 by Steve Lhomme at 2023-11-17T18:37:42+00:00
ttml: only check the visible size changes on the destination

We don't use any other fields from the destination. No need to redo the regions
if other things changed in the source.

On the first call the previous visible dimensions are 0x0.

- - - - -


13 changed files:

- include/vlc_subpicture.h
- modules/access/bluray.c
- modules/codec/arib/libaribcaption.c
- modules/codec/arib/substext.h
- modules/codec/kate.c
- modules/codec/libass.c
- modules/codec/substext.h
- modules/codec/ttml/imageupdater.h
- modules/spu/subsdelay.c
- src/misc/subpicture.c
- src/video_output/video_epg.c
- src/video_output/video_text.c
- src/video_output/video_widgets.c


Changes:

=====================================
include/vlc_subpicture.h
=====================================
@@ -177,8 +177,8 @@ struct vlc_spu_updater_ops
       * the main job of creating the subpicture regions for the
       * current video_format */
     void (*update)(subpicture_t *,
-                   bool has_src_changed, const video_format_t *p_fmt_src,
-                   bool has_dst_changed, const video_format_t *p_fmt_dst,
+                   const video_format_t *prev_src, const video_format_t *p_fmt_src,
+                   const video_format_t *prev_dst, const video_format_t *p_fmt_dst,
                    vlc_tick_t);
 
     /** Optional callback for subpicture private data cleanup */


=====================================
modules/access/bluray.c
=====================================
@@ -1631,12 +1631,12 @@ static void updater_unlock_overlay(bluray_spu_updater_sys_t *p_upd_sys)
 }
 
 static void subpictureUpdaterUpdate(subpicture_t *p_subpic,
-                                    bool b_fmt_src, const video_format_t *p_fmt_src,
-                                    bool b_fmt_dst, const video_format_t *p_fmt_dst,
+                                    const video_format_t *prev_src, const video_format_t *p_fmt_src,
+                                    const video_format_t *prev_dst, const video_format_t *p_fmt_dst,
                                     vlc_tick_t i_ts)
 {
-    VLC_UNUSED(p_fmt_src);
-    VLC_UNUSED(p_fmt_dst);
+    VLC_UNUSED(prev_src); VLC_UNUSED(p_fmt_src);
+    VLC_UNUSED(prev_dst); VLC_UNUSED(p_fmt_dst);
     VLC_UNUSED(i_ts);
 
     bluray_spu_updater_sys_t *p_upd_sys = p_subpic->updater.sys;


=====================================
modules/codec/arib/libaribcaption.c
=====================================
@@ -69,8 +69,6 @@ typedef struct
 {
     decoder_sys_t         *p_dec_sys;
     vlc_tick_t             i_pts;
-    unsigned               i_render_area_width;
-    unsigned               i_render_area_height;
     aribcc_render_result_t render_result;
 } libaribcaption_spu_updater_sys_t;
 
@@ -116,21 +114,24 @@ static void CopyImageToRegion(picture_t *dst_pic, const aribcc_image_t *image)
 }
 
 static void SubpictureUpdate(subpicture_t *p_subpic,
-                             bool b_src_changed, const video_format_t *p_src_format,
-                             bool b_dst_changed, const video_format_t *p_dst_format,
+                             const video_format_t *prev_src, const video_format_t *p_src_format,
+                             const video_format_t *prev_dst, const video_format_t *p_dst_format,
                              vlc_tick_t i_ts)
 {
     libaribcaption_spu_updater_sys_t *p_spusys = p_subpic->updater.sys;
     decoder_sys_t *p_sys = p_spusys->p_dec_sys;
 
+    bool b_src_changed = p_src_format->i_visible_width  != prev_src->i_visible_width ||
+                         p_src_format->i_visible_height != prev_src->i_visible_height;
+    bool b_dst_changed = !video_format_IsSimilar(prev_dst, p_dst_format);
+
+    unsigned i_render_area_width  = p_dst_format->i_visible_width;
+    unsigned i_render_area_height = p_src_format->i_visible_height * p_dst_format->i_visible_width /
+                                    p_src_format->i_visible_width;
     if (b_src_changed || b_dst_changed) {
-        const video_format_t *fmt = p_dst_format;
         /* don't let library freely scale using either the min of width or height ratio */
-        p_spusys->i_render_area_width = fmt->i_visible_width;
-        p_spusys->i_render_area_height = p_src_format->i_visible_height * fmt->i_visible_width /
-                                         p_src_format->i_visible_width;
-        aribcc_renderer_set_frame_size(p_sys->p_renderer, p_spusys->i_render_area_width,
-                                                          p_spusys->i_render_area_height);
+        aribcc_renderer_set_frame_size(p_sys->p_renderer, i_render_area_width,
+                                                          i_render_area_height);
     }
 
     const vlc_tick_t i_stream_date = p_spusys->i_pts + (i_ts - p_subpic->i_start);
@@ -154,25 +155,24 @@ static void SubpictureUpdate(subpicture_t *p_subpic,
 
     vlc_spu_regions_Clear( &p_subpic->regions );
 
-    video_format_t  fmt = *p_dst_format;
-    fmt.i_chroma         = VLC_CODEC_RGBA;
-    fmt.i_x_offset       = 0;
-    fmt.i_y_offset       = 0;
-
     aribcc_image_t *p_images = p_spusys->render_result.images;
     uint32_t        i_image_count = p_spusys->render_result.image_count;
 
-    p_subpic->i_original_picture_width = p_spusys->i_render_area_width;
-    p_subpic->i_original_picture_height = p_spusys->i_render_area_height;
-
     if (!p_images || i_image_count == 0) {
         return;
     }
 
+    p_subpic->i_original_picture_width = i_render_area_width;
+    p_subpic->i_original_picture_height = i_render_area_height;
+
+    video_format_t fmt_region = *p_dst_format;
+    fmt_region.i_chroma       = VLC_CODEC_RGBA;
+    fmt_region.i_x_offset     = 0;
+    fmt_region.i_y_offset     = 0;
+
     /* Allocate the regions and draw them */
     for (uint32_t i = 0; i < i_image_count; i++) {
         aribcc_image_t *image = &p_images[i];
-        video_format_t  fmt_region = fmt;
 
         fmt_region.i_width =
         fmt_region.i_visible_width  = image->width;


=====================================
modules/codec/arib/substext.h
=====================================
@@ -43,21 +43,22 @@ typedef struct arib_text_region_s
 typedef struct
 {
     arib_text_region_t *p_region;
+    bool               first;
 } arib_spu_updater_sys_t;
 
 static void SubpictureTextUpdate(subpicture_t *subpic,
-                                 bool has_src_changed, const video_format_t *fmt_src,
-                                 bool has_dst_changed, const video_format_t *fmt_dst,
+                                 const video_format_t *prev_src, const video_format_t *fmt_src,
+                                 const video_format_t *prev_dst, const video_format_t *fmt_dst,
                                  vlc_tick_t ts)
 {
     arib_spu_updater_sys_t *sys = subpic->updater.sys;
     VLC_UNUSED(fmt_src); VLC_UNUSED(ts);
-    VLC_UNUSED(has_src_changed);
+    VLC_UNUSED(prev_src); VLC_UNUSED(prev_dst);
 
-    if (!has_dst_changed)
+    if ( !sys->first )
         return;
 
-    vlc_spu_regions_Clear( &subpic->regions );
+    assert( vlc_spu_regions_is_empty( &subpic->regions ) );
 
     if (fmt_dst->i_sar_num <= 0 || fmt_dst->i_sar_den <= 0)
     {
@@ -100,6 +101,7 @@ static void SubpictureTextUpdate(subpicture_t *subpic,
         }
         r->p_text->style->i_spacing = p_region->i_horint;
     }
+    sys->first = false;
 }
 static void SubpictureTextDestroy(subpicture_t *subpic)
 {
@@ -128,6 +130,8 @@ static inline subpicture_t *decoder_NewSubpictureText(decoder_t *decoder)
         .destroy  = SubpictureTextDestroy,
     };
 
+    sys->first = true;
+
     subpicture_updater_t updater = {
         .sys = sys,
         .ops = &spu_ops,


=====================================
modules/codec/kate.c
=====================================
@@ -800,8 +800,8 @@ static void PostprocessTigerImage( plane_t *p_plane, unsigned int i_width )
    Looks good with white though since it's all luma. Hopefully that will be the
    common case. */
 static void TigerUpdateSubpicture( subpicture_t *p_subpic,
-                                   bool b_fmt_src, const video_format_t *p_fmt_src,
-                                   bool b_fmt_dst, const video_format_t *p_fmt_dst,
+                                   const video_format_t *prev_src, const video_format_t *p_fmt_src,
+                                   const video_format_t *prev_dst, const video_format_t *p_fmt_dst,
                                    vlc_tick_t ts )
 {
     kate_spu_updater_sys_t *p_spusys = p_subpic->updater.sys;
@@ -811,7 +811,8 @@ static void TigerUpdateSubpicture( subpicture_t *p_subpic,
     kate_float t = (p_spusys->i_start + ts - p_subpic->i_start ) / 1000000.0f;
 
     bool new_regions = false;
-    if( b_fmt_src || b_fmt_dst )
+    if( !video_format_IsSimilar(prev_src, p_fmt_src) ||
+        !video_format_IsSimilar(prev_dst, p_fmt_dst) )
         new_regions = true;
 
     if (!new_regions)


=====================================
modules/codec/libass.c
=====================================
@@ -95,8 +95,8 @@ static void DecSysHold( decoder_sys_t *p_sys );
 
 /* */
 static void SubpictureUpdate( subpicture_t *,
-                              bool, const video_format_t *,
-                              bool, const video_format_t *,
+                              const video_format_t *, const video_format_t *,
+                              const video_format_t *, const video_format_t *,
                               vlc_tick_t );
 static void SubpictureDestroy( subpicture_t * );
 
@@ -417,22 +417,22 @@ static int DecodeBlock( decoder_t *p_dec, block_t *p_block )
  *
  ****************************************************************************/
 static void SubpictureUpdate( subpicture_t *p_subpic,
-                              bool b_fmt_src, const video_format_t *p_fmt_src,
-                              bool b_fmt_dst, const video_format_t *p_fmt_dst,
+                              const video_format_t *prev_src, const video_format_t *p_fmt_src,
+                              const video_format_t *prev_dst, const video_format_t *p_fmt_dst,
                               vlc_tick_t i_ts )
 {
     libass_spu_updater_sys_t *p_spusys = p_subpic->updater.sys;
     decoder_sys_t *p_sys = p_spusys->p_dec_sys;
 
+    bool b_fmt_src = p_fmt_src->i_visible_width  != prev_src->i_visible_width ||
+                     p_fmt_src->i_visible_height != prev_src->i_visible_height;
+    bool b_fmt_dst = !video_format_IsSimilar(prev_dst, p_fmt_dst);
+
     vlc_mutex_lock( &p_sys->lock );
 
-    video_format_t fmt = *p_fmt_dst;
-    fmt.i_chroma         = VLC_CODEC_RGBA;
-    fmt.i_x_offset       = 0;
-    fmt.i_y_offset       = 0;
     if( b_fmt_src || b_fmt_dst )
     {
-        ass_set_frame_size( p_sys->p_renderer, fmt.i_visible_width, fmt.i_visible_height );
+        ass_set_frame_size( p_sys->p_renderer, p_fmt_dst->i_visible_width, p_fmt_dst->i_visible_height );
 #if LIBASS_VERSION > 0x01010000
         ass_set_storage_size( p_sys->p_renderer, p_fmt_src->i_visible_width, p_fmt_src->i_visible_height );
 #endif
@@ -457,8 +457,8 @@ static void SubpictureUpdate( subpicture_t *p_subpic,
     vlc_spu_regions_Clear( &p_subpic->regions );
 
     /* */
-    p_subpic->i_original_picture_height = fmt.i_visible_height;
-    p_subpic->i_original_picture_width = fmt.i_visible_width;
+    p_subpic->i_original_picture_height = p_fmt_dst->i_visible_height;
+    p_subpic->i_original_picture_width = p_fmt_dst->i_visible_width;
 
     /* XXX to improve efficiency we merge regions that are close minimizing
      * the lost surface.
@@ -468,7 +468,7 @@ static void SubpictureUpdate( subpicture_t *p_subpic,
      */
     const int i_max_region = 4;
     rectangle_t region[i_max_region];
-    const int i_region = BuildRegions( region, i_max_region, p_img, fmt.i_width, fmt.i_height );
+    const int i_region = BuildRegions( region, i_max_region, p_img, p_fmt_dst->i_width, p_fmt_dst->i_height );
 
     if( i_region <= 0 )
     {
@@ -477,13 +477,16 @@ static void SubpictureUpdate( subpicture_t *p_subpic,
     }
 
     /* Allocate the regions and draw them */
+    video_format_t fmt_region;
+    fmt_region = *p_fmt_dst;
+    fmt_region.i_chroma   = VLC_CODEC_RGBA;
+    fmt_region.i_x_offset = 0;
+    fmt_region.i_y_offset = 0;
     for( int i = 0; i < i_region; i++ )
     {
         subpicture_region_t *r;
-        video_format_t fmt_region;
 
         /* */
-        fmt_region = fmt;
         fmt_region.i_width =
         fmt_region.i_visible_width  = region[i].x1 - region[i].x0;
         fmt_region.i_height =


=====================================
modules/codec/substext.h
=====================================
@@ -96,14 +96,14 @@ static inline void SubpictureUpdaterSysRegionAdd(substext_updater_region_t *p_pr
 }
 
 static void SubpictureTextUpdate(subpicture_t *subpic,
-                                 bool has_src_changed, const video_format_t *fmt_src,
-                                 bool has_dst_changed, const video_format_t *fmt_dst,
+                                 const video_format_t *prev_src, const video_format_t *fmt_src,
+                                 const video_format_t *prev_dst, const video_format_t *fmt_dst,
                                  vlc_tick_t ts)
 {
     subtext_updater_sys_t *sys = subpic->updater.sys;
-    VLC_UNUSED(fmt_src); VLC_UNUSED(fmt_dst);
 
-    if (!has_src_changed && !has_dst_changed &&
+    if (fmt_src->i_visible_height == prev_src->i_visible_height &&
+        video_format_IsSimilar(prev_dst, fmt_dst) &&
         (sys->i_next_update == VLC_TICK_INVALID || sys->i_next_update > ts))
         return;
 


=====================================
modules/codec/ttml/imageupdater.h
=====================================
@@ -74,16 +74,17 @@ static void TTML_ImageSpuAppendRegion(ttml_image_updater_sys_t *p_sys,
 }
 
 static void TTML_ImageSpuUpdate(subpicture_t *p_spu,
-                                bool b_src_changed, const video_format_t *p_fmt_src,
-                                bool b_dst_changed, const video_format_t *p_fmt_dst,
+                                const video_format_t *prev_src, const video_format_t *p_fmt_src,
+                                const video_format_t *prev_dst, const video_format_t *p_fmt_dst,
                                 vlc_tick_t i_ts)
 {
     VLC_UNUSED(p_fmt_src);
     VLC_UNUSED(i_ts);
-    VLC_UNUSED(b_src_changed);
+    VLC_UNUSED(prev_src);
     ttml_image_updater_sys_t *p_sys = p_spu->updater.sys;
 
-    if (!b_dst_changed)
+    if (p_fmt_dst->i_visible_width  == prev_dst->i_visible_width &&
+        p_fmt_dst->i_visible_height == prev_dst->i_visible_height)
         return;
 
     vlc_spu_regions_Clear( &p_spu->regions );


=====================================
modules/spu/subsdelay.c
=====================================
@@ -214,8 +214,10 @@ static bool SubsdelayIsTextEmpty( const subpicture_region_t * );
  * Subpicture functions
  *****************************************************************************/
 
-static void SubpicUpdateWrapper( subpicture_t *p_subpic, bool has_src_changed, const video_format_t *p_fmt_src,
-                                 bool has_dst_changed, const video_format_t *p_fmt_dst, vlc_tick_t i_ts );
+static void SubpicUpdateWrapper( subpicture_t *p_subpic,
+                                 const video_format_t *prev_src, const video_format_t *p_fmt_src,
+                                 const video_format_t *prev_dst, const video_format_t *p_fmt_dst,
+                                 vlc_tick_t i_ts );
 
 static void SubpicDestroyWrapper( subpicture_t *p_subpic );
 
@@ -914,8 +916,10 @@ static void SubsdelayRecalculateDelays( filter_t *p_filter )
 /*****************************************************************************
  * SubpicUpdateWrapper: Subpicture update callback wrapper
  *****************************************************************************/
-static void SubpicUpdateWrapper( subpicture_t *p_subpic, bool has_src_changed, const video_format_t *p_fmt_src,
-                                 bool has_dst_changed, const video_format_t *p_fmt_dst, vlc_tick_t i_ts )
+static void SubpicUpdateWrapper( subpicture_t *p_subpic,
+                                 const video_format_t *prev_src, const video_format_t *p_fmt_src,
+                                 const video_format_t *prev_dst, const video_format_t *p_fmt_dst,
+                                 vlc_tick_t i_ts )
 {
     subsdelay_heap_entry_t *p_entry;
     vlc_tick_t i_new_ts;
@@ -951,8 +955,8 @@ static void SubpicUpdateWrapper( subpicture_t *p_subpic, bool has_src_changed, c
 
     subpicture_updater_t *updater = &p_entry->p_source->updater;
     updater->ops->update( p_entry->p_source,
-                          has_src_changed, p_fmt_src,
-                          has_dst_changed, p_fmt_dst, i_new_ts );
+                          prev_src, p_fmt_src,
+                          prev_dst, p_fmt_dst, i_new_ts );
 
     p_entry->p_subpic->regions = p_entry->p_source->regions;
 


=====================================
src/misc/subpicture.c
=====================================
@@ -175,10 +175,8 @@ void subpicture_Update( subpicture_t *p_subpicture,
         return;
 
     p_upd->ops->update(p_subpicture,
-                       !video_format_IsSimilar( &p_private->src,
-                                                p_fmt_src ), p_fmt_src,
-                       !video_format_IsSimilar( &p_private->dst,
-                                                p_fmt_dst ), p_fmt_dst,
+                       &p_private->src, p_fmt_src,
+                       &p_private->dst, p_fmt_dst,
                        i_ts);
 
     video_format_Clean( &p_private->src );


=====================================
src/video_output/video_epg.c
=====================================
@@ -495,15 +495,15 @@ static void vout_BuildOSDEpg(epg_spu_updater_sys_t *p_sys,
 }
 
 static void OSDEpgUpdate(subpicture_t *subpic,
-                         bool has_src_changed, const video_format_t *fmt_src,
-                         bool has_dst_changed, const video_format_t *fmt_dst,
+                         const video_format_t *prev_src, const video_format_t *fmt_src,
+                         const video_format_t *prev_dst, const video_format_t *fmt_dst,
                          vlc_tick_t ts)
 {
     epg_spu_updater_sys_t *sys = subpic->updater.sys;
     VLC_UNUSED(fmt_src); VLC_UNUSED(ts);
-    VLC_UNUSED(has_src_changed);
+    VLC_UNUSED(prev_src);
 
-    if (!has_dst_changed)
+    if (video_format_IsSimilar(prev_dst, fmt_dst))
         return;
 
     vlc_spu_regions_Clear( &subpic->regions );


=====================================
src/video_output/video_text.c
=====================================
@@ -38,15 +38,15 @@ typedef struct {
 } osd_spu_updater_sys_t;
 
 static void OSDTextUpdate(subpicture_t *subpic,
-                          bool has_src_changed, const video_format_t *fmt_src,
-                          bool has_dst_changed, const video_format_t *fmt_dst,
+                          const video_format_t *prev_src, const video_format_t *fmt_src,
+                          const video_format_t *prev_dst, const video_format_t *fmt_dst,
                           vlc_tick_t ts)
 {
     osd_spu_updater_sys_t *sys = subpic->updater.sys;
     VLC_UNUSED(fmt_src); VLC_UNUSED(ts);
-    VLC_UNUSED(has_src_changed);
+    VLC_UNUSED(prev_src);
 
-    if (!has_dst_changed)
+    if (video_format_IsSimilar(prev_dst, fmt_dst))
         return;
 
     vlc_spu_regions_Clear( &subpic->regions );


=====================================
src/video_output/video_widgets.c
=====================================
@@ -254,16 +254,16 @@ typedef struct {
 } osdwidget_spu_updater_sys_t;
 
 static void OSDWidgetUpdate(subpicture_t *subpic,
-                            bool has_src_changed, const video_format_t *fmt_src,
-                            bool has_dst_changed, const video_format_t *fmt_dst,
+                            const video_format_t *prev_src, const video_format_t *fmt_src,
+                            const video_format_t *prev_dst, const video_format_t *fmt_dst,
                             vlc_tick_t ts)
 {
     osdwidget_spu_updater_sys_t *sys = subpic->updater.sys;
     subpicture_region_t *p_region;
     VLC_UNUSED(fmt_src); VLC_UNUSED(ts);
-    VLC_UNUSED(has_src_changed);
+    VLC_UNUSED(prev_src);
 
-    if (!has_dst_changed)
+    if (video_format_IsSimilar(prev_dst, fmt_dst))
         return;
 
     vlc_spu_regions_Clear( &subpic->regions );



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/69815da42906f0448d11824f89c79bf8aba6b67d...6928d7658452692ac36c386f3c3b6178b7f838f0

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/69815da42906f0448d11824f89c79bf8aba6b67d...6928d7658452692ac36c386f3c3b6178b7f838f0
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