Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Pr multi refs encode #3266

Merged
merged 2 commits into from
Apr 20, 2020
Merged

Pr multi refs encode #3266

merged 2 commits into from
Apr 20, 2020

Conversation

fulinjie
Copy link
Contributor

Found some potential issues while enabling multi refs encoding in FFmpeg.
Please help to comment.

@fulinjie
codec/encoder: fix multi refs encoding support for FFmpeg
6a6cb82 
1. pCodingParam.iNumRefFrame should be valid within the range of
(MIN_REF_PIC_COUNT, MAX_REF_PIC_COUNT), inclusive.

2. While in WelsBuildRefList(), we should not break after finishing
the first RefList;

3. pCurSliceHeader->uiRefCount should be allowed to equal to
pCurLayer->sLayerInfo.pSpsP->iNumRefFrames.

Signed-off-by: Linjie Fu <[email protected]>
@fulinjie
codec/encoder: loose the reference check
8e90a27 
Allow prompting a warning like:

>>Warning:iNumRefFrame(3) setting does not support the temporal
>>and LTR setting, will be reset to 4

instead of reporting unsupported and fail.

Signed-off-by: Linjie Fu <[email protected]>
fulinjie added a commit to fulinjie/ffmpeg that referenced this pull request Apr 10, 2020
@fulinjie
lavc/libopenh264enc: add multi reference encoding support
3243f8b 
Supports up to 4 refs encoding.

The allowed maximun refs is determined by iTemporalLayerNum
in WelsCheckNumRefSetting().

uiGopSize     = 1 << (iTemporalLayerNum - 1);
iNeededRefNum = WELS_MAX (1, (pParam->uiGopSize >> 1));

iTemporalLayerNum   uiGopSize   iNeededRefNum(supported max)
        1               1            1
        2               2            1
        3               4            2
        4               8            4

Blocked by:
cisco/openh264#3266

Signed-off-by: Linjie Fu <[email protected]>
fulinjie added a commit to fulinjie/ffmpeg that referenced this pull request Apr 11, 2020
@fulinjie
lavc/libopenh264enc: add multi reference encoding support
819ad69 
Supports up to 4 refs encoding.

The allowed maximun refs is determined by iTemporalLayerNum
in WelsCheckNumRefSetting().

uiGopSize     = 1 << (iTemporalLayerNum - 1);
iNeededRefNum = WELS_MAX (1, (pParam->uiGopSize >> 1));

iTemporalLayerNum   uiGopSize   iNeededRefNum(supported max)
        1               1            1
        2               2            1
        3               4            2
        4               8            4

Blocked by:
cisco/openh264#3266

Signed-off-by: Linjie Fu <[email protected]>
@fulinjie
Copy link
Contributor Author

Hi @ruil2 , would you please help to review and offer some comments?

@huili2
Copy link
Collaborator

huili2 commented Apr 13, 2020

the first commit is OK from my side. I'm wondering the "loosing" reason? May it affect the original reference picture settings in hierarchical structures?

@fulinjie
Copy link
Contributor Author

The main reason for loosing this check is to allow automatically adjusting the settings of reference number inside libopenh264 with a warning prompted for user, instead of failing directly.

IMHO this would be more robust/compatible for users if they didn't know the constraints of reference number and keep the encoder working.

For example, if setting -refs 3 in FFmpeg (require reference to 3),
libopenh264 would report warnings like:
iNumRefFrame(3) setting does not support the temporal and LTR setting, will be reset to 4.

After reset the reference, encoder could work well.

Ps, I'm not strongly insisting on this, if it breaks something I didn't noticed.
Feel free to only apply the first commit which is functional, thx.

fulinjie added a commit to fulinjie/ffmpeg that referenced this pull request Apr 15, 2020
@fulinjie
lavc/libopenh264enc: add multi reference encoding support
07ef219 
Supports up to 4 refs encoding.

The allowed maximun refs is determined by iTemporalLayerNum
in WelsCheckNumRefSetting().

uiGopSize     = 1 << (iTemporalLayerNum - 1);
iNeededRefNum = WELS_MAX (1, (pParam->uiGopSize >> 1));

iTemporalLayerNum   uiGopSize   iNeededRefNum(supported max)
        1               1            1
        2               2            1
        3               4            2
        4               8            4

Blocked by:
cisco/openh264#3266

Signed-off-by: Linjie Fu <[email protected]>
@fulinjie
Copy link
Contributor Author

ping.

@fulinjie
Copy link
Contributor Author

Hi @huili2, any updates?

@huili2
Copy link
Collaborator

huili2 commented Apr 20, 2020

sorry late for this. I'm OK for this PR.

@huili2 huili2 merged commit 79a1c1e into cisco:master Apr 20, 2020
@fulinjie
Copy link
Contributor Author

Thanks for the review.

@mstorsjo
Copy link
Contributor

This change broke 18 tests in EncodeFile/EncoderOutputTest - please update the test references accordingly.

mstorsjo added a commit to mstorsjo/openh264 that referenced this pull request Apr 20, 2020
@mstorsjo
Fix test references after pull request cisco#3266
fd92219
@mstorsjo
Copy link
Contributor

Just FWIW, I sent a fix for the test references in #3272.

huili2 added a commit that referenced this pull request Apr 22, 2020
@huili2
Merge pull request #3272 from mstorsjo/fix-test-refs
a547371 
Fix test references after pull request #3266
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fulinjie @huili2 @mstorsjo