[libcamera-devel] libamera: pipeline: rkisp1: timeline: Fix compilation with gcc-[56]

Message ID 20191011143027.23310-1-laurent.pinchart@ideasonboard.com
State Accepted
Commit f3c53dbf53b60defc61948cdfb64f79e6983e071
Headers show
Series
  • [libcamera-devel] libamera: pipeline: rkisp1: timeline: Fix compilation with gcc-[56]
Related show

Commit Message

Laurent Pinchart Oct. 11, 2019, 2:30 p.m. UTC
With gcc 5 and 6, insertion in a std::multimap copies the pair passed as
an argument to the insert() method. As the mapped type is a non-copyable
std::unique_ptr<>, this fails to compile.

Compilation with newer gcc versions succeed due to support for C++-17
and the fix described in https://cplusplus.github.io/LWG/issue2354. To
support gcc 5 and 6, fix the issue by using std::multimap::emplace().

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/pipeline/rkisp1/timeline.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Niklas Söderlund Oct. 11, 2019, 3:46 p.m. UTC | #1
Hi Laurent,

Thanks for spotting this.

On 2019-10-11 17:30:27 +0300, Laurent Pinchart wrote:
> With gcc 5 and 6, insertion in a std::multimap copies the pair passed as
> an argument to the insert() method. As the mapped type is a non-copyable
> std::unique_ptr<>, this fails to compile.
> 
> Compilation with newer gcc versions succeed due to support for C++-17
> and the fix described in https://cplusplus.github.io/LWG/issue2354. To
> support gcc 5 and 6, fix the issue by using std::multimap::emplace().
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  src/libcamera/pipeline/rkisp1/timeline.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/timeline.cpp b/src/libcamera/pipeline/rkisp1/timeline.cpp
> index b98a16689fa9..f6c6434d7b53 100644
> --- a/src/libcamera/pipeline/rkisp1/timeline.cpp
> +++ b/src/libcamera/pipeline/rkisp1/timeline.cpp
> @@ -123,7 +123,7 @@ void Timeline::scheduleAction(std::unique_ptr<FrameAction> action)
>  			<< ", run now " << utils::time_point_to_string(now);
>  		action->run();
>  	} else {
> -		actions_.insert({ deadline, std::move(action) });
> +		actions_.emplace(deadline, std::move(action));
>  		updateDeadline();
>  	}
>  }
> -- 
> Regards,
> 
> Laurent Pinchart
>
Kieran Bingham Oct. 15, 2019, 10:38 a.m. UTC | #2
Re $SUBJECT:

s/libamera/libcamera/

But I'm too late... it's already merged. Ooops oh well.

Perhaps we should add some sort of commit title checking to
checkstyle.py ... but that might be overkill.

Getting spell checking in there should cover it.

--
Kieran


On 11/10/2019 15:30, Laurent Pinchart wrote:
> With gcc 5 and 6, insertion in a std::multimap copies the pair passed as
> an argument to the insert() method. As the mapped type is a non-copyable
> std::unique_ptr<>, this fails to compile.
> 
> Compilation with newer gcc versions succeed due to support for C++-17
> and the fix described in https://cplusplus.github.io/LWG/issue2354. To
> support gcc 5 and 6, fix the issue by using std::multimap::emplace().
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/pipeline/rkisp1/timeline.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/timeline.cpp b/src/libcamera/pipeline/rkisp1/timeline.cpp
> index b98a16689fa9..f6c6434d7b53 100644
> --- a/src/libcamera/pipeline/rkisp1/timeline.cpp
> +++ b/src/libcamera/pipeline/rkisp1/timeline.cpp
> @@ -123,7 +123,7 @@ void Timeline::scheduleAction(std::unique_ptr<FrameAction> action)
>  			<< ", run now " << utils::time_point_to_string(now);
>  		action->run();
>  	} else {
> -		actions_.insert({ deadline, std::move(action) });
> +		actions_.emplace(deadline, std::move(action));
>  		updateDeadline();
>  	}
>  }
>

Patch

diff --git a/src/libcamera/pipeline/rkisp1/timeline.cpp b/src/libcamera/pipeline/rkisp1/timeline.cpp
index b98a16689fa9..f6c6434d7b53 100644
--- a/src/libcamera/pipeline/rkisp1/timeline.cpp
+++ b/src/libcamera/pipeline/rkisp1/timeline.cpp
@@ -123,7 +123,7 @@  void Timeline::scheduleAction(std::unique_ptr<FrameAction> action)
 			<< ", run now " << utils::time_point_to_string(now);
 		action->run();
 	} else {
-		actions_.insert({ deadline, std::move(action) });
+		actions_.emplace(deadline, std::move(action));
 		updateDeadline();
 	}
 }