obs-qsv: Enable QSV texture-based encoding

[brittneysclark]

Enables a pipeline for texture-based encoding with QSV. Utilizes OBS
NV12 output for encode to avoid offloading them from GPU, which will
increase performance. The option to select old QSV pipeline still
remains and will fallback if new pipeline fails.

[Fenrirthviti]

Great to see! Do you know if PR #1609 will work with this new implementation?

We are discussing ways to reduce the number of encoders shown, and rescaling output seems to be the only reason (given that this properly falls back to the old implementation) to show both.

[Gol-D-Ace]

Gave this a quick try.

Compared to the new NVENC implementation there seems to be no fallback to the old method while using advanced output mode. There seems to be fallback in simple output mode though.
Or is simple output mode not even using the new implementation?

The log file also apparently doesn't tell which implementation gets used like with NVENC.

[YaoxinShi]

The fallback code is here:

static void *obs_qsv_create_tex(obs_data_t *settings, obs_encoder_t *encoder)
{
if (obs_nv12_tex_active()) {
return obs_qsv_create(settings, encoder);
} else {
return obs_encoder_create_rerouted(encoder, "obs_qsv11");
}
}

[Gol-D-Ace]

Sorry I guess I was not clear enough.
I didn't meant NV12 being available but rather OBS not running on the Intel GPU.

[YaoxinShi]

@ Gol-D-Ace
The second patch in this PR fix the issue you mention.
A GPU checking code is added during QSV plugin creation. If not on Intel GPU, it will fall back to old QSV plugin.

[brittneysclark]

The continuous integration test says there are conflicts with the second commit, but I cannot see what they are. Is this something I need to address? How can I tell what the problem is?

[Fenrirthviti]

There have been other changes to the same QSV files, so you need to rebase your branch against current master to resolve the conflicts.

[YaoxinShi]

Dear reviewers,

Any comments about this patch? Can it be merged to master now?

Thanks, Yaoxin

[kkartaltepe]

I think we generally want to avoid merge commits in PRs.

If you drop the merge commit then rebase the PR onto master with the 'theirs' merge strategy you should be mostly good. Then just go back and edit each commit with clang format. (you can do both steps during the rebase by rebasing interactive with edit mode and the 'theirs' merge strategy).

[YaoxinShi]

@kkartaltepe, from below check result, this change has no conflicts with the master branch now.

[YaoxinShi]

Hi,

Any help needed to merge this patch to master? It can help to improve and performance a lot and make 4k streaming ready for intel QSV.

Thanks, Yaoxin

[RytoEX]

@YaoxinShi
As @kkartaltepe and @Fenrirthviti said earlier, we generally want to avoid merge commits in PRs.

Rebase the commits onto the current master branch, and make sure the commits are formatted correctly with the project's clang format settings.

[Fenrirthviti]

Just a reminder that this PR will still need to be re-based. It is not mergable in the current state.

Please re-base to remove the merge commits from the PR. If you need assistance with how to do this, please let us know.

[brittneysclark]

@Fenrirthviti -- yes, please help! What is the best way to rebase and remove merge commits?

[RytoEX]

yes, please help! What is the best way to rebase and remove merge commits?

@brittneysclark
In your local Git repo, if you have your fork (brittneysclark/obs-studio) set as a remote named "origin" and obsproject/obs-studio set as a separate remote named "upstream" (you can check your remotes by executing git remote -v), then the following should get you to a cleaner state:

git checkout master
git fetch upstream
git pull --ff-only upstream master
git checkout qsv_texture_based_encoding
git rebase master
git push -f origin

After this, you will still have to merge your fixup commit's contents into the appropriate earlier commits.

[YaoxinShi]

@RytoEX, seems the Linux build failure is an environment issue. Do you know how to solve this?

E: Failed to fetch https://packages.microsoft.com/ubuntu/16.04/prod/dists/xenial/InRelease Clearsigned file isn't valid, got 'NOSPLIT' (does the network require authentication?)

Thanks, Yaoxin

[Fenrirthviti]

You can ignore those build failures, it's an issue with Azure's apt system.

[RytoEX]

@YaoxinShi @brittneysclark
Please use the prefix "obs-qsv11" for commits to the obs-qsv11 plugin.

Hopefully the CI platforms will cooperate on the next build.

[YaoxinShi]

@brittneysclark , can you help on the commit info change?

Thanks, Yaoxin

[dmitryermilov]

Can this copy be avoided? Both m_outBitstream and m_pTaskPool[m_nFirstSyncTask].mfxBS are managed by outside MSDK so application for sure has a chance to pass m_pTaskPool[m_nFirstSyncTask].mfxBS as encoded target and avoid temporal m_outBitstream.

[dmitryermilov]

I know code like

sts = EncodeFrameAsync();
if (MFX_WRN_DEVICE_BUSY == sts)
  MSDK_SLEEP(1);

is widely used in ffmpeg but there is a better solution. In most of the cases MFX_WRN_DEVICE_BUSY means component(decoder/vpp/encoder) "overfeeding". The best way to handle is is to check whether app has not yet handled syncPoint and if so call SyncOperation, like this

sts = EncodeFrameAsync();
if (MFX_WRN_DEVICE_BUSY == sts) {
  if (last_sync_point) {
    SyncOperation(last_sync_point);
    last_sync_point = NULL;
  } else {
  MSDK_SLEEP(1);
  }
}

In most of the cases it will avoid polling and reduce CPU usage.

[dmitryermilov]

Being a MediaSDK developer, I can say this is wrong/bad approach. mfxvideo.h is an external dependency, you can't control it. If tomorrow official MSDK API gets CopyTex function , you'll never be able to use latest MSDK headers/packages.

[dmitryermilov]

Can Intel Gfx be the 1st adapter (not zero one)?

[mufunyo]

DG1 on an AMD APU system in theory?

[dmitryermilov]

yes

[jp9000]

When I made the new NVENC implementation, the biggest mistake was splitting it to "(new)". Instead, it should have just fell back automatically internally. I've provided a patch for you to try out which treats it more seamlessly so the user just gets the best option whenever available without having to configure it:

diff --git a/plugins/obs-qsv11/obs-qsv11.c b/plugins/obs-qsv11/obs-qsv11.c
index 0eb4a3925..f11095820 100644
--- a/plugins/obs-qsv11/obs-qsv11.c
+++ b/plugins/obs-qsv11/obs-qsv11.c
@@ -112,12 +112,6 @@ static const char *obs_qsv_getname(void *type_data)
 	return "QuickSync H.264";
 }
 
-static const char *obs_qsv_getname_tex(void *type_data)
-{
-	UNUSED_PARAMETER(type_data);
-	return "QuickSync H.264 (new)";
-}
-
 static void obs_qsv_stop(void *data);
 
 static void clear_data(struct obs_qsv *obsqsv)
@@ -657,17 +651,23 @@ static void *obs_qsv_create_tex(obs_data_t *settings, obs_encoder_t *encoder)
 	if (!is_intel_gpu_primary()) {
 		blog(LOG_INFO,
 		     ">>> app not on intel GPU, fall back to old qsv encoder");
-		return obs_encoder_create_rerouted(encoder, "obs_qsv11");
+		return obs_encoder_create_rerouted(encoder, "obs_qsv11_soft");
 	}
 
-	if (obs_nv12_tex_active()) {
-		blog(LOG_INFO, ">>> new qsv encoder");
-		return obs_qsv_create(settings, encoder);
-	} else {
+	if (!obs_nv12_tex_active()) {
 		blog(LOG_INFO,
 		     ">>> nv12 tex not active, fall back to old qsv encoder");
-		return obs_encoder_create_rerouted(encoder, "obs_qsv11");
+		return obs_encoder_create_rerouted(encoder, "obs_qsv11_soft");
+	}
+
+	if (obs_encoder_scaling_enabled(encoder)) {
+		blog(LOG_INFO,
+		     ">>> encoder scaling active, fall back to old qsv encoder");
+		return obs_encoder_create_rerouted(encoder, "obs_qsv11_soft");
 	}
+
+	blog(LOG_INFO, ">>> new qsv encoder");
+	return obs_qsv_create(settings, encoder);
 }
 
 static bool obs_qsv_extra_data(void *data, uint8_t **extra_data, size_t *size)
@@ -924,7 +924,7 @@ static bool obs_qsv_encode_tex(void *data, uint32_t handle, int64_t pts,
 }
 
 struct obs_encoder_info obs_qsv_encoder = {
-	.id = "obs_qsv11",
+	.id = "obs_qsv11_soft",
 	.type = OBS_ENCODER_VIDEO,
 	.codec = "h264",
 	.get_name = obs_qsv_getname,
@@ -937,21 +937,22 @@ struct obs_encoder_info obs_qsv_encoder = {
 	.get_extra_data = obs_qsv_extra_data,
 	.get_sei_data = obs_qsv_sei,
 	.get_video_info = obs_qsv_video_info,
-	.caps = OBS_ENCODER_CAP_DYN_BITRATE,
+	.caps = OBS_ENCODER_CAP_DYN_BITRATE | OBS_ENCODER_CAP_INTERNAL,
 };
 
 struct obs_encoder_info obs_qsv_encoder_tex = {
-	.id = "obs_qsv11_tex",
+	.id = "obs_qsv11",
 	.type = OBS_ENCODER_VIDEO,
 	.codec = "h264",
-	.get_name = obs_qsv_getname_tex,
+	.get_name = obs_qsv_getname,
 	.create = obs_qsv_create_tex,
 	.destroy = obs_qsv_destroy,
-	.caps = OBS_ENCODER_CAP_PASS_TEXTURE,
+	.caps = OBS_ENCODER_CAP_DYN_BITRATE | OBS_ENCODER_CAP_PASS_TEXTURE,
 	.encode_texture = obs_qsv_encode_tex,
 	.update = obs_qsv_update,
 	.get_properties = obs_qsv_props,
 	.get_defaults = obs_qsv_defaults,
 	.get_extra_data = obs_qsv_extra_data,
 	.get_sei_data = obs_qsv_sei,
-	.get_video_info = obs_qsv_video_info};
+	.get_video_info = obs_qsv_video_info,
+};

Could you try this out and let me know if this is okay? You'll notice "(new)" no longer shows up, instead it automatically tries to use texture-based encoding if available, otherwise just falls back automatically.

[jp9000]

Oh, and one thing I noticed is that you left out OBS_ENCODER_CAP_DYN_BITRATE from your new structure. I don't think that was intended so I added it back in. If that was intended, let me know.

[YaoxinShi]

@jp9000
Thanks for the comments. I have tried above modification on 3 Intel platforms and it works well.
-- Yaoxin