Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/av/commands/avconv.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ def filter_rotate degrees
when 270
add_output_param vf: 'cclock'
end

self
end

end
Expand Down
21 changes: 21 additions & 0 deletions lib/av/commands/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,27 @@ def parse_param param
end
list
end

def metadata_rotate degrees
add_output_param :'metadata:s:v:0', "rotate=#{degrees}"

self
end

def filter_metadata_rotate degrees
filter_rotate degrees

if @source
current_rotate = identify(@source)[:rotate] || 0
else
::Av.log "Source has not been set - assuming current rotation metadata is 0"
current_rotate = 0
end

metadata_rotate (current_rotate - degrees) % 360
Copy link
Member

Choose a reason for hiding this comment

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

Can we make this clockwise instead of counterclockwise?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've been doing some experiments with this and whilst I can see why the suggestion is to set the rotation information clockwise, this doesn't actually have the expected effect. I wonder if the rotation metadata is actually intended to convey how to play the video instead of conveying how it was rotated.

If I take a landscape video with no rotate metadata, then rotate it 90 degrees clockwise, and also set the metadata to 90 to show its been rotated 90 degrees - quicktime, vlc, ffplay all show the video upside down - ie 90 degrees rotation + another 90 for (the wrong) correction

Conversely if I take that video and set the metadata to 270, it plays correctly in Quicktime, VLC, ffplay?

Copy link
Member

Choose a reason for hiding this comment

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

I guess we won't set the metadata as long as players will rotate the video.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The desktop video players need the metadata in order to decide how much to rotate by. Conversely web based players ignore the metadata (I believe).

I think the rotate metadata describes the 'device (phone) rotation' which is the opposite of the video contents rotation.

Copy link
Member

Choose a reason for hiding this comment

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

Understood. What about the CW rotation (as opposed to the current CCW) implementation?

Copy link
Member

Choose a reason for hiding this comment

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

AFAIU, the way rotation works right now is by rotating minus given degrees (CCW), no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've not changed the rotation method (besides fixing the input/output params issue).

My understanding is that filter_rotate rotates clockwise.

Metadata_rotate sets the metadata to whatever you tell it.

Filter_metadata_rotate rotates the image by the desired amount, then corrects the meta data. If the image is rotated 90 CW, then the metadata should show the recording device as having been rotate 90 CCW, ie 270

Copy link
Member

Choose a reason for hiding this comment

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

I got this all wrong. Merging right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAICT the metadata shows device rotation not image rotation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thx


self
end
end
end
end
2 changes: 2 additions & 0 deletions lib/av/commands/ffmpeg.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def filter_volume vol

def filter_rotate degrees
raise ::Av::InvalidFilterParameter unless degrees % 90 == 0

case degrees
when 90
add_output_param vf: 'transpose=1'
Expand All @@ -37,6 +38,7 @@ def filter_rotate degrees
when 270
add_output_param vf: 'transpose=2'
end

self
end
end
Expand Down
18 changes: 18 additions & 0 deletions spec/av/base_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,23 @@
it { expect { subject.run }.not_to raise_exception }

end

describe '.metadata_rotate' do
before do
subject.metadata_rotate(90)
end

it { expect(subject.output_params.to_s).to eq '-metadata:s:v:0 rotate=90' }
end

describe '.filter_metadata_rotate' do
before do
subject.add_source(source)
subject.filter_metadata_rotate(90)
end

it { expect(subject.output_params.to_s).to match /-metadata:s:v:0 rotate=270/ }
it { expect(subject.output_params.to_s).to match /-vf (clock|transpose=1)/ }
end
end