Message ID | 1461986229-11949-1-git-send-email-ismael@iodev.co.uk (mailing list archive) |
---|---|
State | Superseded, archived |
Delegated to: | Hans Verkuil |
Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.84_2) (envelope-from <linux-media-owner@vger.kernel.org>) id 1awLVZ-00041T-RX; Sat, 30 Apr 2016 03:23:53 +0000 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.76/mailfrontend-5) with esmtp id 1awLVX-0000qH-7J; Sat, 30 Apr 2016 05:23:52 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752356AbcD3DXq (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Fri, 29 Apr 2016 23:23:46 -0400 Received: from iodev.co.uk ([82.211.30.53]:33958 "EHLO iodev.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752300AbcD3DXq (ORCPT <rfc822;linux-media@vger.kernel.org>); Fri, 29 Apr 2016 23:23:46 -0400 X-Greylist: delayed 384 seconds by postgrey-1.27 at vger.kernel.org; Fri, 29 Apr 2016 23:23:45 EDT Received: from localhost (unknown [186.153.93.20]) by iodev.co.uk (Postfix) with ESMTPSA id 90C821F0CF; Fri, 29 Apr 2016 23:15:53 -0400 (EDT) From: Ismael Luceno <ismael@iodev.co.uk> To: linux-media@vger.kernel.org Cc: Hans Verkuil <hverkuil@xs4all.nl>, Ismael Luceno <ismael@iodev.co.uk> Subject: [PATCH 1/2] solo6x10: Set FRAME_BUF_SIZE to 200KB Date: Sat, 30 Apr 2016 00:17:08 -0300 Message-Id: <1461986229-11949-1-git-send-email-ismael@iodev.co.uk> X-Mailer: git-send-email 2.8.0 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2016.4.30.31516 X-PMX-Spam: Gauge=IIIIIIII, Probability=8%, Report=' MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_1200_1299 0, BODY_SIZE_2000_LESS 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, NO_URI_HTTPS 0, SINGLE_URI_IN_BODY 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __CP_URI_IN_BODY 0, __HAS_FROM 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __MIME_TEXT_ONLY 0, __MULTIPLE_RCPTS_CC_X2 0, __SANE_MSGID 0, __SINGLE_URI_TEXT 0, __SUBJ_ALPHA_NEGATE 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_IN_BODY 0, __URI_NO_WWW 0, __URI_NS , __URI_WITH_PATH 0' |
Commit Message
Ismael Luceno
April 30, 2016, 3:17 a.m. UTC
Such frame size is met in practice. Also report oversized frames.
Based on patches by Andrey Utkin <andrey.utkin@corp.bluecherry.net>.
Signed-off-by: Ismael Luceno <ismael@iodev.co.uk>
---
drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
Comments
On Sat, Apr 30, 2016 at 12:17:08AM -0300, Ismael Luceno wrote: > Such frame size is met in practice. Also report oversized frames. > > Based on patches by Andrey Utkin <andrey.utkin@corp.bluecherry.net>. If it is based on my patches([1] [2]), then why you claim authorship and why you don't let me know (at last CCing me)? Do you know that 200 KiB is not the limit, just as previous value? I haven't researched subj deep enough to figure out proven good value for new buffer size. It's both laughable and infuriating for me to spectate your behaviour of "stealth driver developer". You have added yourself back to driver maintainers in MAINTAINERS file after your quit without letting us know. You are not affiliated with Bluecherry for two years, and you are not informed about how the driver is working in production on customers setups. So you are not aware what are real issues with it. BTW do you still have a sample of actual hardware? Yeah, I agree that this can be argument against Bluecherry and lack of openness in its bug tracking. But you are also not open and not collaborating. The point of my accusation to you is that you seem to be just gaining "kernel developer" score for nobody's (except your CV's) benefit. Development and maintenance is what Hans Verkuil, Krzysztof Halasa and others do to this driver, but not this. Sorry to be harsh. [1] https://github.com/bluecherrydvr/solo6x10/commit/5cd985087362e2e524b3e44504eea791ae7cda7e [2] https://github.com/bluecherrydvr/solo6x10/commit/3b437f79c40438eb09bb2d5dbcfe67dbc94648ed -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
OK, I've dropped Ismael's patch from my pull request to Mauro until this is resolved. Who actually maintains this driver? I would say Andrey since he works for bluecherry. If Ismael is no longer affiliated (either officially or unofficially) with bluecherry, then his name should be removed. BTW, looking at the MAINTAINERS file I see two email addresses for Andrey, neither of which is the fastmail.com address this email came from. Ismael, please keep the correct author if it isn't you. And a CC to Andrey certainly doesn't hurt. Andrey, it might be a good idea to post such fixes to the mailinglist sooner, both to prevent situations like this and to keep the diffs between mainline and your internal code as small as possible. I don't really care who posts fixes as long as authorship info etc. remains intact and the code is obviously an improvement. Regards, Hans On 05/04/2016 03:34 PM, Andrey Utkin wrote: > On Sat, Apr 30, 2016 at 12:17:08AM -0300, Ismael Luceno wrote: >> Such frame size is met in practice. Also report oversized frames. >> >> Based on patches by Andrey Utkin <andrey.utkin@corp.bluecherry.net>. > > If it is based on my patches([1] [2]), then why you claim authorship and > why you don't let me know (at last CCing me)? > > Do you know that 200 KiB is not the limit, just as previous value? I > haven't researched subj deep enough to figure out proven good value for > new buffer size. > > It's both laughable and infuriating for me to spectate your behaviour of > "stealth driver developer". > You have added yourself back to driver maintainers in MAINTAINERS file > after your quit without letting us know. > You are not affiliated with Bluecherry for two years, and you are not > informed about how the driver is working in production on customers > setups. So you are not aware what are real issues with it. BTW do you > still have a sample of actual hardware? Yeah, I agree that this can be > argument against Bluecherry and lack of openness in its bug tracking. > But you are also not open and not collaborating. > > The point of my accusation to you is that you seem to be just gaining > "kernel developer" score for nobody's (except your CV's) benefit. > Development and maintenance is what Hans Verkuil, Krzysztof Halasa and > others do to this driver, but not this. > > Sorry to be harsh. > > [1] https://github.com/bluecherrydvr/solo6x10/commit/5cd985087362e2e524b3e44504eea791ae7cda7e > [2] https://github.com/bluecherrydvr/solo6x10/commit/3b437f79c40438eb09bb2d5dbcfe67dbc94648ed > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, May 4, 2016 at 5:04 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote: > BTW, looking at the MAINTAINERS file I see two email addresses for Andrey, > neither of which is the fastmail.com address this email came from. Now I'm replying from corporate email. > Andrey, it might be a good idea to post such fixes to the mailinglist sooner, > both to prevent situations like this and to keep the diffs between mainline > and your internal code as small as possible. In a word - we would do what is possible to achieve that, but there's little time and little incentive for that. The codebases have already diverged a lot, having unique sets of runtime bugs. And this exact issue alone is not resolved yet in a good way and is not actually critical. Merging would require a lot of working time. And it is complicated by the fact that there's not going to be any new manufacturing orders (the minimal order quantity is too high for Bluecherry), and that we have picked tw5864 as reachable for retail orders. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/May/2016 16:34, Andrey Utkin wrote: > On Sat, Apr 30, 2016 at 12:17:08AM -0300, Ismael Luceno wrote: > > Such frame size is met in practice. Also report oversized frames. > > > > Based on patches by Andrey Utkin <andrey.utkin@corp.bluecherry.net>. > > If it is based on my patches([1] [2]), then why you claim authorship and > why you don't let me know (at last CCing me)? Wasn't my intention, I gave you credit, I just merged the changes and reworked the warning and commit message. The whole point to have solo6x10 mainlined was to get rid of the out-of-tree driver and convert the DKMS package to use media_build, thus mainline should be kept in sync, so why are you not submitting the patches yourself? I should have nothing to do with that. > Do you know that 200 KiB is not the limit, just as previous value? I > haven't researched subj deep enough to figure out proven good value for > new buffer size. I know, I know it depends on the quantization matrix, and it should be possible to infer the limit, but like you I didn't do the research, the difference is that I don't get paid to do it anymore. > It's both laughable and infuriating for me to spectate your behaviour of > "stealth driver developer". > You have added yourself back to driver maintainers in MAINTAINERS file > after your quit without letting us know. Why the attack? I didn't quit, I was dismissed, and the remaining ~5k USD I'm owed was never paid. Curtis: any comment on that? Also, I don't see what's the problem in re-adding myself, and I don't understand why I was removed in the first place, it's not up to Bluecherry, is it? > You are not affiliated with Bluecherry for two years, and you are not > informed about how the driver is working in production on customers > setups. So you are not aware what are real issues with it. BTW do you > still have a sample of actual hardware? Yeah, I agree that this can be > argument against Bluecherry and lack of openness in its bug tracking. You attend Bluecherry customers' needs because you're part of Bluecherry; like you said I'm not, and I certainly don't get paid to care about the out-of-tree driver issues or it's bug tracker. And yes, I still have the hardware. > But you are also not open and not collaborating. What do you think I should do? Seriously, I don't get it. > The point of my accusation to you is that you seem to be just gaining > "kernel developer" score for nobody's (except your CV's) benefit. > Development and maintenance is what Hans Verkuil, Krzysztof Halasa and > others do to this driver, but not this. > > Sorry to be harsh. I think you're the only one keeping such a score, and I never claimed my work being more or superior to anyone's else. You're out of your mind, man. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/May/2016 17:22, Andrey Utkin wrote: > On Wed, May 4, 2016 at 5:04 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote: > > BTW, looking at the MAINTAINERS file I see two email addresses for Andrey, > > neither of which is the fastmail.com address this email came from. > > Now I'm replying from corporate email. > > > Andrey, it might be a good idea to post such fixes to the mailinglist sooner, > > both to prevent situations like this and to keep the diffs between mainline > > and your internal code as small as possible. > > In a word - we would do what is possible to achieve that, but > there's little time and little incentive for that. > The codebases have already diverged a lot, having unique sets of runtime bugs. The divergence is due to the vb2 porting effort by Hans. History was not properly conserved, but it is otherwise compatible, and most changes can still be ported back and forth without trouble. > And this exact issue alone is not resolved yet in a good way and is > not actually critical. > Merging would require a lot of working time. And it is complicated by > the fact that there's not going to be any new manufacturing orders > (the minimal order quantity is too high for Bluecherry), and that > we have picked tw5864 as reachable for retail orders. Right now it would be easy to keep the two in sync, there has been not many changes since I left, and if changes are integrated into mainline first, there will be no further issues. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/04/2016 04:22 PM, Andrey Utkin wrote: > On Wed, May 4, 2016 at 5:04 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote: >> BTW, looking at the MAINTAINERS file I see two email addresses for Andrey, >> neither of which is the fastmail.com address this email came from. > > Now I'm replying from corporate email. > >> Andrey, it might be a good idea to post such fixes to the mailinglist sooner, >> both to prevent situations like this and to keep the diffs between mainline >> and your internal code as small as possible. > > In a word - we would do what is possible to achieve that, but there's > little time > and little incentive for that. > The codebases have already diverged a lot, having unique sets of runtime bugs. > And this exact issue alone is not resolved yet in a good way and is > not actually critical. > Merging would require a lot of working time. And it is complicated by > the fact that > there's not going to be any new manufacturing orders (the minimal order quantity > is too high for Bluecherry), and that we have picked tw5864 as > reachable for retail orders. > It sounds more like the status of this driver is "Odd Fixes" and not "Supported" as it says today. From the MAINTAINERS file: S: Status, one of the following: Supported: Someone is actually paid to look after this. Maintained: Someone actually looks after it. Odd Fixes: It has a maintainer but they don't have time to do much other than throw the odd patch in. See below.. Orphan: No current maintainer [but maybe you could take the role as you write your new code]. Obsolete: Old code. Something tagged obsolete generally means it has been replaced by a better system and you should be using that. Yes, Ismael should have kept your authorship, but it won't be the first time someone forgets that. Heck, I've forgotten it on occasion. Just point that out politely and let Ismael post a new version of this patch. Don't look for conspiracies when it is just a mistake. Relax, peace on earth, don't worry, be happy and all that. Bad for everyone's blood pressure to get so worked up about these things. Looking at the recent history of this driver the patch contributions seem to be more-or-less equally distributed between Krzysztof, Andrey and Ismael, not that there are all that many. So if Ismael is merging in some of your patches for free in his own time, then I'd say why not? Again, provided correct authorship is maintained. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c index 67a14c4..f98017b 100644 --- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c +++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c @@ -33,7 +33,7 @@ #include "solo6x10-jpeg.h" #define MIN_VID_BUFFERS 2 -#define FRAME_BUF_SIZE (196 * 1024) +#define FRAME_BUF_SIZE (200 * 1024) #define MP4_QS 16 #define DMA_ALIGN 4096 @@ -323,8 +323,11 @@ static int solo_send_desc(struct solo_enc_dev *solo_enc, int skip, int i; int ret; - if (WARN_ON_ONCE(size > FRAME_BUF_SIZE)) + if (WARN_ON_ONCE(size > FRAME_BUF_SIZE)) { + dev_warn(&solo_dev->pdev->dev, + "oversized frame (%d bytes)\n", size); return -EINVAL; + } solo_enc->desc_count = 1;