[FFmpeg-devel] [PATCH v2] avformat/dvdvideodec: Simplify padding cell trim logic

[Marth64]

When -trim option is used (by default), padding cells
at the beginning of the title are supposed to be ignored.
The current implementation does the ignoring when we
attempt to lock on to the PGC navigation event stream,
but this is a wrong assumption because we have already
entered the PGC. This causes false positives on some
discs due to a search for a program stream cell that
never succeeds.

User would have to know to disable the -trim option
to work around the issue.

Simplify the logic and move it to the MPEG-PS block
reading event handling, in turn also fixing the trim
function for these weirdly authored discs.

Signed-off-by: Marth64 <marth64 at proxyid.net>
---
 libavformat/dvdvideodec.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/libavformat/dvdvideodec.c b/libavformat/dvdvideodec.c
index e7132725b7..f1c960b56b 100644
--- a/libavformat/dvdvideodec.c
+++ b/libavformat/dvdvideodec.c
@@ -624,7 +624,6 @@ static int dvdvideo_play_next_ps_block(AVFormatContext *s, DVDVideoPlaybackState
     dvdnav_vts_change_event_t *e_vts;
     dvdnav_cell_change_event_t *e_cell;
     int cur_title, cur_pgcn, cur_pgn, cur_angle, cur_title_unused, cur_ptt, cur_nb_angles;
-    int is_cell_promising = 0;
     pci_t *e_pci;
     dsi_t *e_dsi;
 
@@ -706,23 +705,17 @@ static int dvdvideo_play_next_ps_block(AVFormatContext *s, DVDVideoPlaybackState
                     continue;
 
                 e_cell = (dvdnav_cell_change_event_t *) nav_buf;
-                is_cell_promising = !c->opt_trim || dvdvideo_is_cell_promising(s, state->pgc, e_cell->cellN);
 
-                av_log(s, AV_LOG_DEBUG, "new cell: prev=%d new=%d promising=%d\n",
-                                        state->celln, e_cell->cellN, is_cell_promising);
+                av_log(s, AV_LOG_DEBUG, "new cell: prev=%d new=%d\n", state->celln, e_cell->cellN);
 
                 if (!state->in_ps && !state->in_pgc) {
                     if (cur_title == c->opt_title                        &&
                         (c->opt_pgc || cur_ptt == c->opt_chapter_start)  &&
                         cur_pgcn == state->pgcn                          &&
-                        cur_pgn == state->entry_pgn                      &&
-                        is_cell_promising) {
+                        cur_pgn == state->entry_pgn) {
 
                         state->in_pgc = 1;
                     }
-
-                    if (c->opt_trim && !is_cell_promising)
-                        av_log(s, AV_LOG_INFO, "Skipping padding cell #%d\n", e_cell->cellN);
                 } else if (state->celln >= e_cell->cellN || state->pgn > cur_pgn) {
                     return AVERROR_EOF;
                 }
@@ -784,9 +777,13 @@ static int dvdvideo_play_next_ps_block(AVFormatContext *s, DVDVideoPlaybackState
 
                 return nav_len;
             case DVDNAV_BLOCK_OK:
-                if (!state->in_ps) {
-                    if (state->in_pgc)
-                        i = 0; /* necessary in case we are skipping junk cells at the beginning */
+                if (!state->in_ps)
+                    continue;
+
+                if (c->opt_trim && !dvdvideo_is_cell_promising(s, state->pgc, state->celln)) {
+                    av_log(s, AV_LOG_DEBUG, "skipping block in trimmed cell #%d\n", state->celln);
+
+                    i = 0;
                     continue;
                 }
 
-- 
2.34.1