-
Notifications
You must be signed in to change notification settings - Fork 168
Vbv dev #155
Vbv dev #155
Conversation
…istance 2. Added Check for FrameIndex not exceding No of Frame to Encode
… the feedback of the previous base layer has arrived. The impact on speed to be evaluated
…r than or equal to the TemporalId of the access unit containing the NAL unit
This fix ensures right signalling of Picture timing for keyframes>0
…usly It was going beyond target Bitrate and was within Max Bitrate, if Max rate is Greater than Target Bitrate
Source/App/EbAppConfig.c
Outdated
| static void SetVbvBufsize (const char *value, EbConfig_t *cfg) { cfg->vbvBufsize = strtoul(value, NULL, 0); }; | ||
| static void SetVbvBufInit (const char *value, EbConfig_t *cfg) { cfg->vbvBufInit = strtoul(value, NULL, 0); }; | ||
| static void SetHrdFlag (const char *value, EbConfig_t *cfg) { cfg->hrdFlag = strtoul(value, NULL, 0); }; | ||
| static void SetVbvMaxrate (const char *value, EbConfig_t *cfg) { cfg->vbvMaxRate = strtoul(value, NULL, 0); }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove the necessary space here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified code as per review comments and pushed patch (Commit : 9db1593)
Source/API/EbApi.h
Outdated
| // VBV Parameters | ||
|
|
||
| /* Sets the maximum rate the VBV buffer should be assumed to refill at | ||
| * Default is zero */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the indent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved and pushed patch (Commit : 9db1593)
| | **vbvBufsize** | -vbv-bufsize | Any Number | 0 | VBV BufferSize in bits / second. Only used when RateControlMode is set to 1 | | ||
| | **vbvBufInit** | -vbv-init | [0 - 100] | 90 | Sets how full the VBV buffer to be| | ||
| | **hrdFlag** | -hrd | [0,1] | 0 | HRD Flag, 0 = OFF, 1 = ON |When hrdFlag is set to 1 it requires vbvMaxrate and vbvBufsize to be greater than 0 | | ||
| | **MaxQpAllowed** | -max-qp | [0 - 51] | 48 | Maximum QP value allowed for rate control use. Only used when RateControlMode is set to 1. Has to be >= MinQpAllowed | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also update sample.cfg? Thx.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated and pushed (Commit : 9db1593)
README.md
Outdated
| [](https://travis-ci.com/OpenVisualCloud/SVT-HEVC) | ||
| [](https://coveralls.io/github/openvisualcloud/SVT-HEVC?branch=master) | ||
|
|
||
| [](https://ci.appveyor.com/project/intel/SVT-HEVC) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The left side openvisualcloud is the new one, please fix these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed and pushed patch (Commit : 9db1593)
README.md
Outdated
| - [svt-hevc-encoder-user-guide](Docs/svt-hevc_encoder_user_guide.md) | ||
|
|
||
| - [SVT-HEVC Encoder User Guide](Docs/svt-hevc_encoder_user_guide.md) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove extra empty line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed empty line and pushed patch (Commit : 9db1593)
Source/Lib/Codec/EbEntropyCoding.c
Outdated
| bitstreamPtr, | ||
| scsPtr->staticConfig.fpsInVps == 1 ? EB_TRUE : EB_FALSE); | ||
| if (scsPtr->staticConfig.fpsInVps == 1) | ||
| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if() {
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified and pushed (Commit: 9db1593)
Source/Lib/Codec/EbEntropyCoding.c
Outdated
| MIN_PU_SIZE, | ||
| &countNonZeroCoeffs); | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove extra line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified and pushed (Commit: 9db1593)
Source/Lib/Codec/EbEntropyCoding.c
Outdated
| MIN_PU_SIZE, | ||
| &countNonZeroCoeffs); | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove extra line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified and pushed (Commit: 9db1593)
Source/Lib/Codec/EbIntraPrediction.c
Outdated
| switch(lumaMode) { | ||
|
|
||
| case EB_INTRA_PLANAR: | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove extra lines in this file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified and pushed (Commit: 9db1593)
| referenceEntryPtr->releaseEnable = EB_TRUE; | ||
| referenceEntryPtr->referenceAvailable = EB_FALSE; | ||
| referenceEntryPtr->isUsedAsReferenceFlag = pictureControlSetPtr->isUsedAsReferenceFlag; | ||
| referenceEntryPtr->feedbackArrived = EB_FALSE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix indent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified and pushed (Commit: 9db1593)
-Updated code as per review comments as per coding guidelines, Indendation etc
| { | ||
| printf("Error Instance %u: hrdFlag must be [0 - 1]\n", channelNumber + 1); | ||
| printf("SVT [Error]: Instance %u: hrdFlag must be [0 - 1]\n", channelNumber + 1); | ||
| return_error = EB_ErrorBadParameter; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use "SVT_LOG("SVT [Error]:"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please follow the coding style here.
if () {
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified and pushed (Commit: 9db1593)
Source/Lib/Codec/EbEncHandle.c
Outdated
| if (config->hrdFlag == 1 && ((config->vbvBufsize <= 0) || (config->vbvMaxrate <= 0))) | ||
| { | ||
| printf("Error instance %u: hrd requires vbv max rate and vbv bufsize to be greater than 0 ", channelNumber + 1); | ||
| printf("SVT [Error]: Instance %u: hrd requires vbv max rate and vbv bufsize to be greater than 0 ", channelNumber + 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use "SVT_LOG("SVT [Error]:"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified and pushed (Commit: 9db1593)
Source/App/EbAppConfig.c
Outdated
| { SINGLE_INPUT, USE_QP_FILE_TOKEN, "UseQpFile", SetCfgUseQpFile }, | ||
| { SINGLE_INPUT, RATE_CONTROL_ENABLE_TOKEN, "RateControlMode", SetRateControlMode }, | ||
| { SINGLE_INPUT, LOOK_AHEAD_DIST_TOKEN, "LookAheadDistance", SetLookAheadDistance}, | ||
| { SINGLE_INPUT, TARGET_BIT_RATE_TOKEN, "TargetBitRate", SetTargetBitRate }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove duplicated code here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed Duplicate code, Commit : 5d0af0f
Source/Lib/Codec/EbEncHandle.c
Outdated
|
|
||
| //Set required flags to signal vbv status when hrd is enabled | ||
| if (sequenceControlSetPtr->staticConfig.hrdFlag == 1) { | ||
| sequenceControlSetPtr->staticConfig.videoUsabilityInfo = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the indent of your code changes in this file. Use space instead of tab.
Please review carefully the diff here, and fix all of them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified code , Used space in place of tab, reviewed full EbEncHandle.c file, Commit : 5d0af0f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @dinesh0603 Sorry that I can't approve this commit. As you can see here, the indents are still messed up compared with original code.
Please also fix the indents in Source/API/EbApi.h. Read this file on github, you will know what is the difference. Please keep it same with original code.
Source/App/EbAppConfig.c
Outdated
|
|
||
| // Quantization | ||
| { SINGLE_INPUT, QP_TOKEN, "QP", SetCfgQp }, | ||
| { SINGLE_INPUT, USE_QP_FILE_TOKEN, "UseQpFile", SetCfgUseQpFile }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UseQpFile is duplicated. Pls clean up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed duplicate code and pushed code (commit:67f68d8)
|
Hi @dinesh0603, could you also provide some test commands or examples to demonstrate the usage of the new APIs? |
|
@dinesh0603 , just tried to run vbv hrd(-hrd 1) test on my machine, but the generated bitstream gets crash for HM decoder, could you help take a look if there are errors on my cmdline? BTW, the same test with -hrd 0 works well. |
SEIPeriod issues Signed-off-by: Hu, Kelvin <[email protected]>
Fixed hrd 1 crash issues caused by writeUvlc long bits, memmove and
Signed-off-by: kirithika <[email protected]>
Signed-off-by: kirithika <[email protected]>
…into vbv_dev # Conflicts: # Source/Lib/Codec/EbEncHandle.c # Source/Lib/Codec/EbEntropyCoding.c # Source/Lib/Codec/EbPacketizationProcess.c
This commit cleans up the unused variables in VBV code and add file changes that got missed out while resolving conflicts Signed-off-by: kirithika <[email protected]>
|
Hi, @kirithika7, could you add a sign-off line to your commit msg? Thx. |
Hi Amir & Hassene,
Please let me know if you are facing any issues.
Thanks & Regards,
Dinesh