search

OpenVisualCloud / SVT-HEVC

SVT HEVC encoder. Scalable Video Technology (SVT) is a software-based video coding technology that is highly optimized for Intel® Xeon® processors. Using the open source SVT-HEVC encoder, it is possible to spread video encoding processing across multiple Intel® Xeon® processors to achieve a real advantage of processing efficiency.
Other
519 stars 169 forks source link

Question in UpdateHistogramQueueEntry() #439

Closed calvinckfong closed 4 years ago

calvinckfong commented 4 years ago

In UpdateHistogramQueueEntry(), histogramQueueEntryIndex is computed with the following lines:

    histogramQueueEntryIndex = (EB_S32)(pictureControlSetPtr->pictureNumber - encodeContextPtr->hlRateControlHistorgramQueue[encodeContextPtr->hlRateControlHistorgramQueueHeadIndex]->pictureNumber);
    histogramQueueEntryIndex += encodeContextPtr->hlRateControlHistorgramQueueHeadIndex;
    histogramQueueEntryIndex = (histogramQueueEntryIndex > HIGH_LEVEL_RATE_CONTROL_HISTOGRAM_QUEUE_MAX_DEPTH - 1) ?
        histogramQueueEntryIndex - HIGH_LEVEL_RATE_CONTROL_HISTOGRAM_QUEUE_MAX_DEPTH :
        histogramQueueEntryIndex;

It seems to me that it is equivalent to:

histogramQueueEntryIndex = (pictureControlSetPtr->pictureNumber % HIGH_LEVEL_RATE_CONTROL_HISTOGRAM_QUEUE_MAX_DEPTH);

Not just simplifying the code, but also fixed an issue I met when rate control is on. I ran into a case that histogramQueueEntryIndex becomes negative and trigger a core dump during a long run of a few hours. Its occurrence varies from 1 to 26 hours such that it is hard for me to trace the problem. I wanna know the mechanism of the original code and see if there will be any drawbacks in my fix. Thanks!

tianjunwork commented 4 years ago

Hi @calvinckfong, thank you for the question, we will take a look on Monday.

tianjunwork commented 4 years ago

Hi @calvinckfong could you share the command line that caused core dump? We've never seen this issue before and would like to reproduce.

calvinckfong commented 4 years ago

Hi @tianjunwork, after studying for a few days, I found that the above codes are not the root cause.

Let me explain a bit more about my test. I wrapped svt-hevc (v1.3.0) into a gstreamer plugin called svthevcencoder and I called this plugin with the following command:

gst-launch-1.0 -v videotestsrc pattern='snow' ! videoconvert ! videoscale ! video/x-raw,width=848,height=480,framerate=25/1 ! svthevcencoder bitrate=1000 intra-refresh=0 key-int-max=25 rc=1 ! queue ! fakesink

I tested on both i7 and xeon machines and found that xeon crashed easier (maybe just because it runs faster). It crashed within 1 hour. I looked into the code deeper and seems to find the cause. hlRateControlHistorgramQueue is not properly mutex-locked when it is accessed.

I changed some codes with proper mutex lock. It seems running well for 1+ hour. To save your time, let me have a longer test first. Thanks.

tianjunwork commented 4 years ago

Hi @calvinckfong , can I know the reason that you didn't choose to use the gst plugins inside this repo? Sure, let us know your test result:)

calvinckfong commented 4 years ago

Hi @tianjunwork. I modified the gst plugin in this repo to mine. No more error or crash in a 18-hour run. I believe the problem is resolved.

I just checked that this issue has been resolved in #239 . Sorry that bothering your time. I think this issue can be closed. Thanks.

tianjunwork commented 4 years ago

Hi @calvinckfong, no problem:) You are welcome.