FFmpegWriter loses SAR (sample aspect ratio) on output videos

[ferdnyc]

While looking into OpenShot/openshot-qt#3329 some, I decided to export the test video I had set up with the "NTSC 23.98 fps" profile, which is the one with this definition:

description=NTSC 23.98 fps
frame_rate_num=24000
frame_rate_den=1001
width=720
height=486
progressive=1
sample_aspect_num=8
sample_aspect_den=9
display_aspect_num=4
display_aspect_den=3
colorspace=601

However, when I exported the video, I was surprised to discover that despite the Aspect Ratio being set to 4:3 and the Pixel Ratio being set to 8:9 (confirmed on the Advanced / Profile tab), the resulting video detected differently in both VLC and MediaInfo:

$ mediainfo -Full /var/tmp/aspect.mp4|egrep -i '(width|height| ratio )'
Width                                    : 720
Width                                    : 720 pixels
Height                                   : 486
Height                                   : 486 pixels
Stored_Height                            : 496
Sampled_Width                            : 720
Sampled_Height                           : 486
Pixel aspect ratio                       : 1.000
Display aspect ratio                     : 1.481
Display aspect ratio                     : 3:2

Looking through the code, I noticed that while the pixel_ratio Fraction is passed in to SetVideoOptions(), after that:

• It's stored in info.pixel_ratio (as .num and .den)
  

  libopenshot/src/FFmpegWriter.cpp

  Lines 248 to 254 in 166f693

  	if (width >= 1)
  	info.width = width;
  	if (height >= 1)
  	info.height = height;
  	if (pixel_ratio.num > 0) {
  	info.pixel_ratio.num = pixel_ratio.num;
  	info.pixel_ratio.den = pixel_ratio.den;

• It's used to compute info.display_ratio from the width and height:
  

  libopenshot/src/FFmpegWriter.cpp

  Lines 264 to 272 in 166f693

  	// Calculate the DAR (display aspect ratio)
  	Fraction size(info.width * info.pixel_ratio.num, info.height * info.pixel_ratio.den);
  	// Reduce size fraction
  	size.Reduce();
  	// Set the ratio based on the reduced fraction
  	info.display_ratio.num = size.num;
  	info.display_ratio.den = size.den;

...And then it's never used again. Nor is display_ratio! Neither of them are ever even accessed. Neither one is supplied to any FFmpeg context. In fact, other than setting the AVCodecContext width and height:

libopenshot/src/FFmpegWriter.cpp

Lines 1207 to 1210 in 166f693

	/* resolution must be a multiple of two */
	// TODO: require /2 height and width
	c->width = info.width;
	c->height = info.height;

...there's nothing further in the code that sets the output sizing or attempts to define the sample_aspect_ratio of any of the output contexts.

Meanwhile, if I rip DVD content from a 4:3 NTSC DVD, the results are different:

$ # First converting to AVI, because `ffprobe` can't directly examine .VOB files
$ cat VTS*.VOB | ffmpeg -i - -s 720x576 -c:v mpeg4 -c:a mp3 -y VTS.avi

$ # Now, the same MediaInfo command I ran on the OpenShot output file above
$ mediainfo -Full VTS.avi|egrep -i '(width|height| ratio )'
Width                                    : 720
Width                                    : 720 pixels
Height                                   : 576
Height                                   : 576 pixels
Sampled_Width                            : 720
Sampled_Height                           : 576
Pixel aspect ratio                       : 1.067
Display aspect ratio                     : 1.333
Display aspect ratio                     : 4:3

$ # And ffprobe also shows a SAR that isn't 1:1...
$ ffprobe -hide_banner -i VTS.avi            
Input #0, avi, from 'VTS.avi':
  Metadata:
    encoder         : Lavf58.29.100
  Duration: 00:08:34.91, start: 0.000000, bitrate: 598 kb/s
    Stream #0:0: Video: mpeg4 (Simple Profile) (FMP4 / 0x34504D46), yuv420p,
 720x576 [SAR 16:15 DAR 4:3], 456 kb/s, 29.97 fps, 29.97 tbr, 29.97 tbn, 30k tbc
    Stream #0:1: Audio: mp3 (U[0][0][0] / 0x0055), 48000 Hz, stereo, fltp, 128 kb/s

As a result, when I play the VTS.avi file in VLC, it automatically formats itself to 4:3 aspect — switching from "Default" to "4:3" has no discernible effect. However, if I play that aspect.mp4 test export from OpenShot, switching to "4:3" does change the size, because the "Default" presentation is not correctly sized due to the false 1:1 pixel ratio.

Don't we need to be using the info.pixel_ratio to set sample_aspect_ratio on the output context, when writing videos with non-uniform pixel ratios?

cc: @jonoomph @eisneinechse @SuslikV

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[SuslikV]

I prefer to use square pixels and never looked into the non-square ones. Maybe you are right.

[MattLathropTL]

I can confirm SAR is not maintained. I have been resampling each frame to square pixels and changing the resolution of the videos to accommodate in my software.

[hgerry]

Just to say that I would also really appreciate a fix for this as I am coming across the same issue when processing some miniDV footage. I do love this software overall, thanks all for your work.

[stale]

Thank you so much for submitting an issue to help improve OpenShot Video Editor. We are sorry about this, but this particular issue has gone unnoticed for quite some time. To help keep the OpenShot GitHub Issue Tracker organized and focused, we must ensure that every issue is correctly labelled and triaged, to get the proper attention.

This issue will be closed, as it meets the following criteria:

• No activity in the past 90 days
• No one is assigned to this issue

We'd like to ask you to help us out and determine whether this issue should be reopened.

• If this issue is reporting a bug, please can you attempt to reproduce on the latest daily build to help us to understand whether the bug still needs our attention.
• If this issue is proposing a new feature, please can you verify whether the feature proposal is still relevant.

Thanks again for your help!