Frame metadata & sidedata, FilterContext options helpers #173
Conversation
| getOption(name, flags = optionFlags.SEARCH_CHILDREN) { | ||
| return binding.getOption(this._handle, name, flags) | ||
| } | ||
|
|
||
| setOption(name, value, flags = optionFlags.SEARCH_CHILDREN) { | ||
| binding.setOption(this._handle, name, value, flags) | ||
| } | ||
|
|
||
| setOptionDictionary(dictionary, flags = optionFlags.SEARCH_CHILDREN) { | ||
| binding.setOptionDictionary(this._handle, dictionary._handle, flags) | ||
| } | ||
|
|
||
| setOptionDefaults() { | ||
| binding.setOptionDefaults(this._handle) | ||
| } | ||
|
|
||
| listOptionNames(flags = optionFlags.SEARCH_CHILDREN) { | ||
| return binding.listOptionNames(this._handle, flags) | ||
| } | ||
|
|
||
| getOptions(flags = optionFlags.SEARCH_CHILDREN) { | ||
| const options = {} | ||
|
|
||
| for (const name of this.listOptionNames(flags)) { | ||
| try { | ||
| options[name] = this.getOption(name, flags) | ||
| } catch (error) { | ||
| // Non-string options are currently ignored to match CodecContext behaviour. | ||
| } | ||
| } | ||
|
|
||
| return options | ||
| } |
There was a problem hiding this comment.
Maybe this is time for having a class we could compose with?
|
|
||
| class FrameMetadata { | ||
| constructor(frame) { | ||
| this._frame = frame |
There was a problem hiding this comment.
If we only use the frame I would do this:
| this._frame = frame | |
| this._frameHandle = frame._handle |
| get sideData() { | ||
| const handles = binding.getFrameSideData(this._handle) | ||
| return handles.map((handle) => new FrameSideData(handle)) | ||
| } |
There was a problem hiding this comment.
Now that I rethink about it, It looks like we repeat a lot of code with PacketSideData.
Maybe we should open an issue and discuss how we could generalise with a SideData abstract instead of the dupe PacketSideData/FrameSideData
| t.ok(filterCtx) | ||
| }) | ||
|
|
||
| test('FilterContext exposes runtime option helpers', (t) => { |
There was a problem hiding this comment.
I would split the test into two different scenarios. In the first one we assert that get returns something and its type (or that it is the same than default) and in the second we assert the set logic.
In case of regression we could see which part is failing.
| const frame = new ffmpeg.Frame() | ||
|
|
||
| const payload = Buffer.from( | ||
| Array.from({ length: 32 }, (_, index) => (index * 7) & 0xff) | ||
| ) |
There was a problem hiding this comment.
| const frame = new ffmpeg.Frame() | |
| const payload = Buffer.from( | |
| Array.from({ length: 32 }, (_, index) => (index * 7) & 0xff) | |
| ) | |
| const frame = new ffmpeg.Frame() | |
| const payload = Buffer.from( | |
| Array.from({ length: 32 }, (_, index) => (index * 7) & 0xff) | |
| ) |
| frame.removeSideData(ffmpeg.constants.frameSideDataType.CONTENT_LIGHT_LEVEL) | ||
| t.is(frame.sideData.length, 0) |
There was a problem hiding this comment.
I would test that separately.
| const inputFrame = createAudioFrame(0) | ||
| t.teardown(() => { | ||
| inputFrame.destroy() | ||
| }) |
| const entries = outputFrame.metadata.entries() | ||
| t.ok(entries.length > 0) | ||
|
|
||
| const astatsEntry = entries.find(([key]) => key.startsWith('lavfi.astats.')) | ||
| t.ok(astatsEntry) | ||
|
|
||
| const [key, value] = astatsEntry | ||
| t.is(outputFrame.metadata.get(key), value) | ||
| t.is(outputFrame.metadata.get('missing-key'), null) |
There was a problem hiding this comment.
Sound like we asserting distincts things, maybe could we put it into different scenarios.
|
@tony-go Yeah, both the FrameSideData class and the FilterContext options api are duplicating other code at the moment. Should we fix that in this pull request? I was thinking of doing that if/when a third instance of the code is imminent, but it could be done now. I'll look at what else has a side data api and options api. |
@sethvincent As I said here #173 (comment)
We could simply open an issue. |
Co-authored-by: Tony Gorez <gorez.tony@gmail.com>
Co-authored-by: Tony Gorez <gorez.tony@gmail.com>
This adds metadata and sidedata apis to the Frame class and adds options helpers to FilterContext.