Skip to content

Transcode anamorphic video.#602

Merged
subdavis merged 10 commits into
mainfrom
client/non-anamorphic-transcoding
Mar 3, 2021
Merged

Transcode anamorphic video.#602
subdavis merged 10 commits into
mainfrom
client/non-anamorphic-transcoding

Conversation

@subdavis
Copy link
Copy Markdown
Contributor

@subdavis subdavis commented Feb 25, 2021

Pixel aspect ratio !== 1:1 is not supported. Force it in transcoding.

This may slightly alter the actual width/height of the video

https://viame.kitware.com/#/viewer/601a0582503d63fdb4d021f5
https://viame.kitware.com/#viewer/602441854d85a3cb4e2250d2

fixes #570

@subdavis
Copy link
Copy Markdown
Contributor Author

Screenshot from 2021-02-24 23-17-06

Works a lot better when you use SAR 1:1.

Interestingly, the tracker output is different too, so I think even the tracker hates weird SAR values.

@subdavis
Copy link
Copy Markdown
Contributor Author

subdavis commented Feb 25, 2021

I also tried using gcs.

      this.featureLayer.gcs([
        '+proj=longlat',
        '+axis=esd',
        '+s11=2', '+s12=0', '+s13=0',
        '+s21=0', '+s22=1', '+s23=0',
        '+s31=0', '+s32=0', '+s33=1',
      ].join(' '));

Only parts of this work. the +axis bit works on everything. But the rest of the affine transform only works on text features, and is ignored by polygons.

This reinforces my fear of client-side transformations. If we had a regression, we would find out about it much later because this is such a niche use case, and we would have to locate and correct all the corrupted manual annotations that users may have created. This kind of mistake could be very costly in time and customer relationship (since we could have corrupted hours of manual labor in creating annotations)

Imagine if we had to script updates to all line geometry only because some future change cause it to start ignoring this transform.

I also don't like the additional mental hoops I'd have to jump through. "Does 175px really mean 175?"

One thing I can do is distort the video rather than the annotations. This doesn't seem ideal for users, but it seems a lot safer than hoping all layer types treat gcs arguments the same.

Screenshot from 2021-02-25 08-19-15

Screenshot from 2021-02-25 08-22-22

@subdavis subdavis requested a review from BryonLewis February 25, 2021 14:35
@BryonLewis
Copy link
Copy Markdown
Collaborator

did you attempt this on the wrasse.mp4 data? It was giving me an error related to the width being non-divisible by 2. It wants to scale the video from 720x480 to 853x480 but that then is a non-even width.
https://stackoverflow.com/questions/20847674/ffmpeg-libx264-height-not-divisible-by-2
Adding that filter to the end of the scaling resets it to 854x480 and allows it to transcode properly.
I.E: '-vf "scale=iw*sar:ih,setsar=1,pad=ceil(iw/2)*2:ceil(ih/2)*2"',

@subdavis
Copy link
Copy Markdown
Contributor Author

subdavis commented Feb 25, 2021

I have a similar local change that I forgot to push, but they aren't the same.

ceil(iw/2)*2:ceil(ih/2)*2 is not the same as -vf "scale=ceil(iw*sar):ih,setsar=1"

Mine causes the final aspect ratio to be different than the input by some fraction of a pixel width Yours seems like it must be doing the same. My margin of error is 0.5pixel widths, but yours seems like it could be closer to 1 pixel width.

I'm not the best at math.

@BryonLewis
Copy link
Copy Markdown
Collaborator

With a really weird SAR you could end up getting a non-even width.
Source file: 523x222 SAR: 49:33
523*(49/33) = 776.575757 | ceil of that is 777

The padding way would take that 776.5757/2 = 388.2878 | ceil = 389 | double = 778

I think mine will add up to 1.9999999 pixels black letter box like scaling on the ends while maintain aspect ratio, an updated version of your command (ceil(iw*sar/2)*2:ih) will stretch it the additional maximum of 1.99999 pixels. I don't know which one is proper to do.

@BryonLewis
Copy link
Copy Markdown
Collaborator

Update: Just doubled checked and it looks like ffmpeg supports round so you could cut the maximum amount of being off to 1 pixel instead of 2. But then I would limit to squashing the output using your method, don't think you can pad negative space:
'-vf "scale=round(iw*sar/2)*2:ih,setsar=1"',

That would have an output width of 776 instead of the 778 for the weird source video.

Comment thread client/platform/desktop/backend/native/viame.ts
Comment thread client/platform/desktop/constants.ts
'-crf 26',
'-c:a copy',
// https://video.stackexchange.com/questions/20871/how-do-i-convert-anamorphic-hdv-video-to-normal-h-264-video-with-ffmpeg-how-to
'-vf "scale=iw*sar:ih,setsar=1"',
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

determination of how we want to modify this for non-even values.
I like so far -vf "scale=round(iw*sar/2)*2:round(ih/2)*2,setsar=1"
Fixes any issues with height or width being odd by adjusting by at most one pixel in either direction.

Comment thread server/dive_tasks/tasks.py Outdated
"26",
"-c:a",
"copy",
"-vf \"scale=iw*sar:ih,setsar=1\"",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

same non-even width/height issue as the electron versions:
-vf "scale=round(iw*sar/2)*2:round(ih/2)*2,setsar=1"

Update: Of course the version of the ffmpeg in the container doesn't support round

Comment thread server/dive_tasks/tasks.py
@subdavis subdavis requested a review from BryonLewis March 3, 2021 16:36
BryonLewis
BryonLewis previously approved these changes Mar 3, 2021
Copy link
Copy Markdown
Collaborator

@BryonLewis BryonLewis left a comment

Choose a reason for hiding this comment

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

Tested the web version and desktop version with some weird SAR datasets and it works properly. Web has the benefit of converting everything and the desktop was properly detecting x264/mp4s that had non 1:1 SAR values and converting them. I then ran pipelines through the results making sure they were applied to the converted versions.

Comment thread server/dive_tasks/utils.py Outdated
Co-authored-by: BryonLewis <61746913+BryonLewis@users.noreply.github.com>
@subdavis subdavis requested a review from BryonLewis March 3, 2021 18:21
Copy link
Copy Markdown
Collaborator

@BryonLewis BryonLewis left a comment

Choose a reason for hiding this comment

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

did another pass through with my test datasets and all seem to be working in both modes.

@subdavis subdavis merged commit 9c7de00 into main Mar 3, 2021
@subdavis subdavis deleted the client/non-anamorphic-transcoding branch March 3, 2021 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Video display for unusual pixel ratios

2 participants