Closed Bug 1433062 Opened 3 years ago Closed 3 years ago

Write unittest for VP8TrackEncoder's custom keyframe interval

Categories

(Core :: Audio/Video: Recording, enhancement, P3)

58 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: pehrsons, Assigned: pehrsons)

References

Details

Attachments

(4 files)

Assignee: nobody → apehrson
Status: NEW → ASSIGNED
Rank: 25
Priority: -- → P3
Attachment #8945475 - Flags: feedback?(john357smith)
Attachment #8945476 - Flags: feedback?(john357smith)
Attachment #8945477 - Flags: feedback?(john357smith)
Attachment #8945478 - Flags: feedback?(john357smith)
John, you can fetch these to your local repo with mercurial,
`hg pull -r 8bae308855a5fd212804878dd267751009012a5a https://reviewboard-hg.mozilla.org/gecko`

Then run the tests by,
`./mach gtest VP8*KeyFrame*`

Should you need to debug bug 1411857 using these you can,
`./mach gtest VP8*KeyFrame* --debugger gdb`

Debugger depending a bit on platform. If you're on linux I can recommend rr.
Comment on attachment 8945475 [details]
Bug 1433062 - Unit test VP8TrackEncoder with shorter-than-default keyframe interval.

https://reviewboard.mozilla.org/r/215630/#review221356
Attachment #8945475 - Flags: review?(bvandyk) → review+
Comment on attachment 8945476 [details]
Bug 1433062 - Unit test VP8TrackEncoder with longer-than-default keyframe interval.

https://reviewboard.mozilla.org/r/215632/#review221358
Attachment #8945476 - Flags: review?(bvandyk) → review+
Comment on attachment 8945477 [details]
Bug 1433062 - Unit test VP8TrackEncoder with default keyframe interval.

https://reviewboard.mozilla.org/r/215634/#review221360
Attachment #8945477 - Flags: review?(bvandyk) → review+
Comment on attachment 8945478 [details]
Bug 1433062 - Unit test VP8TrackEncoder with dynamic keyframe interval changes.

https://reviewboard.mozilla.org/r/215636/#review221362

Looks good, nice comments detailing keyframe locations and timings ^_^
Attachment #8945478 - Flags: review?(bvandyk) → review+
Comment on attachment 8945475 [details]
Bug 1433062 - Unit test VP8TrackEncoder with shorter-than-default keyframe interval.

With last patchset this test works correctly.
Attachment #8945475 - Flags: feedback?(john357smith) → feedback+
Comment on attachment 8945476 [details]
Bug 1433062 - Unit test VP8TrackEncoder with longer-than-default keyframe interval.

With last patchset this test works correctly.
Attachment #8945476 - Flags: feedback?(john357smith) → feedback+
Comment on attachment 8945477 [details]
Bug 1433062 - Unit test VP8TrackEncoder with default keyframe interval.

There is a problem with not-set keyframe interval. Now code (see patchset) suppose 0 value is not defined value so it skips any manual keyframe forcing and let it on automatic mode which means 600 frames (MAX_KEYFRAME_INTERVAL) has to pass before a keyframe is created.
Attachment #8945477 - Flags: feedback?(john357smith) → feedback?(apehrson)
Comment on attachment 8945478 [details]
Bug 1433062 - Unit test VP8TrackEncoder with dynamic keyframe interval changes.

With last patchset this test works correctly.
Attachment #8945478 - Flags: feedback?(john357smith) → feedback+
Comment on attachment 8945477 [details]
Bug 1433062 - Unit test VP8TrackEncoder with default keyframe interval.

(In reply to john357smith from comment #16)
> Comment on attachment 8945477 [details]
> Bug 1433062 - Unit test VP8TrackEncoder with default keyframe interval.
> 
> There is a problem with not-set keyframe interval. Now code (see patchset)
> suppose 0 value is not defined value so it skips any manual keyframe forcing
> and let it on automatic mode which means 600 frames (MAX_KEYFRAME_INTERVAL)
> has to pass before a keyframe is created.

See bug 1411857 comment 77
Attachment #8945477 - Flags: feedback?(apehrson)
Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0549fd8f2c2b
Unit test VP8TrackEncoder with shorter-than-default keyframe interval. r=bryce
https://hg.mozilla.org/integration/mozilla-inbound/rev/bcb50a75f8fb
Unit test VP8TrackEncoder with longer-than-default keyframe interval. r=bryce
https://hg.mozilla.org/integration/mozilla-inbound/rev/c6ce23184078
Unit test VP8TrackEncoder with default keyframe interval. r=bryce
https://hg.mozilla.org/integration/mozilla-inbound/rev/f31c356e0a5c
Unit test VP8TrackEncoder with dynamic keyframe interval changes. r=bryce
You need to log in before you can comment on or make changes to this bug.