diff mbox series

[FFmpeg-devel,v3] avformat/dvdvideodec: Fix incorrect padding cell trim logic

Message ID 20240702030526.1288022-1-marth64@proxyid.net
State New
Headers show
Series [FFmpeg-devel,v3] avformat/dvdvideodec: Fix incorrect padding cell trim logic | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 fail Make fate failed
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Marth64 July 2, 2024, 3:05 a.m. UTC
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 after we
have locked on to the PGC navigation event stream,
but does not set the PGC/PG state properly.

This causes false positives and errors 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 NAV packet
event handling, in turn implementing the behaviour
correctly and fixing the trim function for impacted discs.

Signed-off-by: Marth64 <marth64@proxyid.net>
---
 libavformat/dvdvideodec.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

Comments

Marth64 July 2, 2024, 6:41 a.m. UTC | #1
Gah. Screwed this one up again. See v4. (This will be the last time).
Sorry, this was a really frustrating one to fix/test.
diff mbox series

Patch

diff --git a/libavformat/dvdvideodec.c b/libavformat/dvdvideodec.c
index e7132725b7..f5b7dd33e5 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;
                 }
@@ -766,6 +759,13 @@  static int dvdvideo_play_next_ps_block(AVFormatContext *s, DVDVideoPlaybackState
                        e_pci->pci_gi.nv_pck_lbn, state->vobu_duration, state->nav_pts);
 
                 if (!state->in_ps) {
+                    if (c->opt_trim && !dvdvideo_is_cell_promising(s, state->pgc, state->celln)) {
+                        av_log(s, AV_LOG_INFO, "Skipping padding cell #%d\n", state->celln);
+
+                        i = 0;
+                        continue;
+                    }
+
                     av_log(s, AV_LOG_DEBUG, "navigation: locked to program stream\n");
 
                     state->in_ps = 1;
@@ -784,11 +784,8 @@  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 (nav_len != DVDVIDEO_BLOCK_SIZE) {
                     av_log(s, AV_LOG_ERROR, "Invalid MPEG block size (expected=%d actual=%d)\n",