Hi Rajat Singhal (@rajat2004),

Thanks for your interest in this PR. My answers are below:

• I used that script and the average FPS is 19.90, don't know how much it is in the regular camera. It does support all kinds of image types, and it is possible to get images and subwindows the same way and updates as the regular camera.

• Well, I think that it's something that the maintainers should answer, if that could be interesting or not. I can talk about my own experience. It opened me to a whole world of possibilities. If you want to use the drone with filming objectives (that it is wide use of the UAVs), this PR is more than helpful because u could not modify the intrinsics of the camera otherwise.

About the corrections, David Zimmermann (@zimmy87) is having a look at the code, so if he considers that, I can correct the code with your suggestions.

By the FPS, I meant some results to compare on your system itself, for the Cinematic and normal cameras, for different resolutions like 256x144, 720p, 1080p and maybe 4k also?
Makes sense, UAVs for filming is probably the most common usecase, other applications are also closely vision-related, and improvements in camera to model the real world would definitely be useful

Our Azure pipeline check is failing with the following build error:

In file included from /__w/13/s/Unreal/Environments/Blocks/Plugins/AirSim/Intermediate/Build/Linux/B4D820EA/UE4/Development/AirSim/Module.AirSim.2_of_2.cpp:2:
/__w/13/s/Unreal/Environments/Blocks/Plugins/AirSim/Source/TextureShuffleActor.cpp:24:49: error: incomplete type 'UMaterialInstanceDynamic' named in nested name specifier
        DynamicMaterialInstances[material_id] = UMaterialInstanceDynamic::Create(DynamicMaterial, this);
                                                ^~~~~~~~~~~~~~~~~~~~~~~~~~
/home/ue4/ue-4.25.1-linux-debugeditor/Engine/Source/Runtime/Engine/Classes/Engine/BlendableInterface.h:10:7: note: forward declaration of 'UMaterialInstanceDynamic'
class UMaterialInstanceDynamic;
      ^
In file included from /__w/13/s/Unreal/Environments/Blocks/Plugins/AirSim/Intermediate/Build/Linux/B4D820EA/UE4/Development/AirSim/Module.AirSim.2_of_2.cpp:2:
/__w/13/s/Unreal/Environments/Blocks/Plugins/AirSim/Source/TextureShuffleActor.cpp:27:33: error: member access into incomplete type 'UStaticMeshComponent'
        components[component_id]->SetMaterial(material_id, DynamicMaterialInstances[material_id]);
                                ^
/home/ue4/ue-4.25.1-linux-debugeditor/Engine/Source/Runtime/CoreUObject/Public/Templates/Casts.h:22:7: note: forward declaration of 'UStaticMeshComponent'
class UStaticMeshComponent;
      ^
In file included from /__w/13/s/Unreal/Environments/Blocks/Plugins/AirSim/Intermediate/Build/Linux/B4D820EA/UE4/Development/AirSim/Module.AirSim.2_of_2.cpp:2:
/__w/13/s/Unreal/Environments/Blocks/Plugins/AirSim/Source/TextureShuffleActor.cpp:30:42: error: member access into incomplete type 'UMaterialInstanceDynamic'
    DynamicMaterialInstances[material_id]->SetTextureParameterValue("TextureParameter", SwappableTextures[tex_id]);
                                         ^
/home/ue4/ue-4.25.1-linux-debugeditor/Engine/Source/Runtime/Engine/Classes/Engine/BlendableInterface.h:10:7: note: forward declaration of 'UMaterialInstanceDynamic'
class UMaterialInstanceDynamic;
      ^
4 errors generated.

Are you seeing this in local builds?

Hi David Zimmermann (@zimmy87), I answered your suggestions/bug reports. I will fix everything once we decide what to do to integrate it.

No, I haven't seen that error of compilation in local builds. I'm using UE 4.26.1 in Ubuntu 20. Can this be the difference?

What are the next steps we should do? Should I wait for your instructions?

Hi Pablo Pueyo Ramon (@ppueyor), apologies for the delay in getting back to you; I've responded to the remaining feedback items; please let me know if you have any other questions/concerns. Regarding the build failure in TextureShuffleActor.cpp, I'm still seeing this with your latest revision, so this looks like a legitimate build failure. My guess is you're missing an include somewhere (usually the "incomplete type" error occurs when only a forward declaration is present for a given class and the full header for that class is needed to access one of its members). We're currently using 4.25.1 on our build machines, and we'd like to maintain compatibility with 4.25, so using 4.26.1 could be a potential cause here. Would you be able to test locally with a 4.25.1 build?

Hi David Zimmermann (@zimmy87),

Thanks for the review. Yes, I will test it in Unreal 4.25.1 once we decide what to do with the cinematic flag in Settings.hpp.
I fixed the suggested changes. I will commit everything when it is all decided and ready.

Hi David Zimmermann (@zimmy87),

I implemented all the requested changes and updated my repo to the latest version of your master.
I talked to Jonny and we decided to set the focal length to match with the regular camera.

I tested it with ubuntu 20 and Unreal 4.25 and works perfectly.

Please, tell me if you need anything else.

P.S. : I needed many commits to bug simple bugs, if you could merge them in one, it will be cleaner, sorry for this

Hi Pablo Pueyo Ramon (@ppueyor), thanks for the update, I tested locally and noticed a few things that I think should get addressed before merging. I've gone through all previous review feedback and resolved everything that looks to have been addressed. I see that there's one remaining item of feedback from Rajat Singhal (@rajat2004) on PythonClient/airsim/client.py that hasn't been addressed yet (it looks like this may not require a code change, feel free to resolve the comment yourself if that's the case). There also looks like there's one remaining item of feedback from my previous review about the names of SrcPPSettings, DstPPSettings, and DstWeightedBlendables in Unreal/Plugins/AirSim/Source/PIPCamera.cpp. These variables should be renamed to src_pp_settings, dst_pp_settings, and dst_weighted_blendables.

Hi David Zimmermann (@zimmy87) and Rajat Singhal (@rajat2004) , I resolved all the changes requested, hope everything is correct now.
There is some docu/demo work to do, but I don't have much time to do it right now. I will do it asap.

The new focal length looks good when I manually override the right camera, but unfortunately I'm not seeing this focal length applied to the default camera, so I still see the same narrow field of view with the latest iteration.

Does the "PostInitializeComponents" thing solve this?

Tested the latest iteration and it works well for me, so I am moving ahead with merging. It would be nice to add some extra documentation and examples, but I feel that can be handled in a separate PR.