Skip to content

Commit

Permalink
Revert "demux_mkv: PAR should be calculated after applying crop"
Browse files Browse the repository at this point in the history
Matroska spec says that DisplayWidth and DisplayHeight should be applied
after cropping, but this doesn't adhere to the real files which does not
follow this rule. Revert the change and we can re-evaluate if someone
complains with spec compliant files.

See: https://datatracker.ietf.org/doc/draft-ietf-cellar-matroska/

This reverts commit f8db02b.
  • Loading branch information
kasper93 authored and Dudemanguy committed Oct 21, 2023
1 parent dc2298f commit 81102b0
Showing 1 changed file with 7 additions and 10 deletions.
17 changes: 7 additions & 10 deletions demux/demux_mkv.c
Original file line number Diff line number Diff line change
Expand Up @@ -1505,23 +1505,20 @@ static int demux_mkv_open_video(demuxer_t *demuxer, mkv_track_t *track)
sh_v->disp_w = track->v_width;
sh_v->disp_h = track->v_height;

struct mp_rect crop;
crop.x0 = track->v_crop_left;
crop.y0 = track->v_crop_top;
crop.x1 = track->v_width - track->v_crop_right;
crop.y1 = track->v_height - track->v_crop_bottom;

// Keep the codec crop rect as 0s if we have no cropping since the
// file may have broken width/height tags.
if (track->v_crop_left || track->v_crop_top ||
track->v_crop_right || track->v_crop_bottom)
{
sh_v->crop = crop;
sh_v->crop.x0 = track->v_crop_left;
sh_v->crop.y0 = track->v_crop_top;
sh_v->crop.x1 = track->v_width - track->v_crop_right;
sh_v->crop.y1 = track->v_height - track->v_crop_bottom;
}

int dw = track->v_dwidth_set ? track->v_dwidth : mp_rect_w(crop);
int dh = track->v_dheight_set ? track->v_dheight : mp_rect_h(crop);
struct mp_image_params p = {.w = mp_rect_w(crop), .h = mp_rect_h(crop)};
int dw = track->v_dwidth_set ? track->v_dwidth : track->v_width;
int dh = track->v_dheight_set ? track->v_dheight : track->v_height;
struct mp_image_params p = {.w = track->v_width, .h = track->v_height};
mp_image_params_set_dsize(&p, dw, dh);
sh_v->par_w = p.p_w;
sh_v->par_h = p.p_h;
Expand Down

2 comments on commit 81102b0

@jmuchemb
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eek! I only have spec compliant files. I followed the specs when I started to use this feature and I was waiting for mpv support for a long time. Meanwhile, I was using a lua script to automatically set a crop filter.

I don't want to remux all of them.

What about an option to either disable this feature (can be useful too and I could find any) or switch between the 2 behaviors ?

/cc @kasper93

@jmuchemb
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

disable this feature (can be useful too and I could find any)

Oops, I missed --video-crop=0x0+0+0:

  • When search the man page, I thought it would not work because this option should be only about cropping whereas the mkv feature is also about aspect ratio.
  • This option would be useful if it applied after the mkv feature.

Please sign in to comment.