[FFmpeg-devel] DVCPRO HD: request for review
Michael Niedermayer
michaelni
Fri Aug 29 19:50:47 CEST 2008
On Fri, Aug 29, 2008 at 09:54:25AM -0700, Roman V. Shaposhnik wrote:
> On Fri, 2008-08-29 at 05:52 +0200, Michael Niedermayer wrote:
> > > I'll commit everything today and this piece
> >
> > Iam not sure if this and your commit of 170k of unapproved code is a joke.
> > If it is, i do not like you humor.
>
> No it isn't. It is an honest attempt to continue as a DV maintainer.
>
> > Please revert it and commit the parts that have been approved cleanly
> > in several seperate commits.
>
> No. And if treating maintainers in this manner is your kind of humor
> I don't think I appreciate it either. Moreover, this is not the first
> time when you ridicule me after I honestly tried to work all the issues
> with you and had an impression that they were all resolved.
They clearly where not resolved, and i do not belive that you seriously
belived that they where. I even told you per private mail that you should
commit the ok-ed parts several times that implicates that there are also
not ok-ed parts.
> I do have
> commitments as far as DVCPRO HD is concerned
I could have guessed that much from the daily private mails from you about
reviewing your patch soon and quickly. And that you would help me where
possible if i just review them quickly.
I told you many things where you could help, like commiting the parts of
the patch ive ok-ed or comiting some of the ok-ed parts of patches of
SOC students or fixing some bugs at roundup. You did _nothing_
you where even too lazy to commit the parts of your OWN patch that i
had approved you just reposted the whole and kept pushing me to review it
again
sorry but in this case "your commitments" seems to be quite a bit in
the way of keeping the code in reassonable clean shape.
> so I'm willing to tolerate
> your behavior while I'm trying to integrate decoder and encoder.
> But once that happens, I believe I have no place in the project like
> this.
That is sad, but i will not tolerate that people start commiting unclean
code because their payment from some contact might depend on it.
> So, please start finding another maintainer for DV stuff. Heavens
> are my witness I tried to work with you. I tried to be helpful to the
> project. I'm not as smart as you are. But I believe I can be helpful.
> You know that I went over the roundup yesterday and saw a couple of
> areas where my puny coding skills are a match. But you, know, I really
> don't need this aggravation...
Where exactly did you do something on roundup? I cannot find a single mail
from you on the corresponding mailing list.
>
> Anyway, now for the technical part...
>
> Yes. I will revert the commit once I get to a place where svn works.
> This likely to happen by COB today. However, I have no clue what you're
> talking about when you say that I have committed unapproved code. As
> far as I'm concerned everything in:
> libavformat/isom.c has been approved
> libavformat/dv.c has been approved
> libavcodec/dv.c has been approved
> the only bit you didn't *explicitly* approved is the tables. And given
> that I explained two times why I am adding them and why they have
> a high chance of going away real soon once I transition to any
> scheme that proves to be faster. It is all documented in the
> thread -- look it up.
Well, so the tables are in svn now, what holds you up with replacing them
now as you promissed in the thread?
>
> Now, as for committing individual changes. There's none to be committed
> (except for isom.c) so that you have a *working* DV codec between the
> commits. I have always though that people are using changesets for a
Some examples:
@@ -59,8 +64,8 @@ typedef struct DVVideoContext {
/* MultiThreading - dv_anchor applies to entire DV codec, not just the avcontext */
/* one element is needed for each video segment in a DV frame */
-/* at most there are 2 DIF channels * 12 DIF sequences * 27 video segments (PAL 50Mbps) */
-#define DV_ANCHOR_SIZE (2*12*27)
+/* at most there are 4 DIF channels * 12 DIF sequences * 27 video segments (1080i50) */
+#define DV_ANCHOR_SIZE (4*12*27)
static void* dv_anchor[DV_ANCHOR_SIZE];
------------------
@@ -51,6 +55,7 @@ typedef struct DVVideoContext {
uint8_t dv_zigzag[2][64];
uint32_t dv_idct_factor[2][2][22][64];
+ uint32_t dv100_idct_factor[4][4][16][64];
void (*get_pixels)(DCTELEM *block, const uint8_t *pixels, int line_size);
void (*fdct[2])(DCTELEM *block);
@@ -102,6 +107,17 @@ static void dv_build_unquantize_tables(D
}
}
}
+
+ for(a = 0; a < 4; a++) {
+ for(q = 0; q < 16; q++) {
+ for(i = 1; i < 64; i++) {
+ s->dv100_idct_factor[0][a][q][i]= (dv100_qstep[q]<<(a+9))*dv_iweight_1080_y[i];
+ s->dv100_idct_factor[1][a][q][i]= (dv100_qstep[q]<<(a+9))*dv_iweight_1080_c[i];
+ s->dv100_idct_factor[2][a][q][i]= (dv100_qstep[q]<<(a+9))*dv_iweight_720_y[i];
+ s->dv100_idct_factor[3][a][q][i]= (dv100_qstep[q]<<(a+9))*dv_iweight_720_c[i];
+ }
+ }
+ }
}
static av_cold int dvvideo_init(AVCodecContext *avctx)
Modified: trunk/libavcodec/dvdata.h
==============================================================================
--- trunk/libavcodec/dvdata.h (original)
+++ trunk/libavcodec/dvdata.h Fri Aug 29 00:41:00 2008
@@ -316,6 +316,13 @@ static const uint8_t dv_quant_shifts[22]
static const uint8_t dv_quant_offset[4] = { 6, 3, 0, 1 };
static const uint8_t dv_quant_areas[4] = { 6, 21, 43, 64 };
+/* quantization quanta by QNO for DV100 */
+static const uint8_t dv100_qstep[16] = {
+ 1, /* QNO = 0 and 1 both have no quantization */
+ 1,
+ 2, 3, 4, 5, 6, 7, 8, 16, 18, 20, 22, 24, 28, 52
+};
+
/* NOTE: I prefer hardcoding the positioning of dv blocks, it is
simpler :-) */
----------------------
@@ -391,6 +410,11 @@ static int dv_read_header(AVFormatContex
return AVERROR(EIO);
c->dv_demux->sys = dv_frame_profile(c->buf);
+ if (!c->dv_demux->sys) {
+ av_log(s, AV_LOG_ERROR, "Can't determine profile of DV input stream.\n");
+ return -1;
+ }
+
s->bit_rate = av_rescale(c->dv_demux->sys->frame_size * 8,
c->dv_demux->sys->frame_rate,
c->dv_demux->sys->frame_rate_base);
-----------------------
And of course there are many more, but iam not too interrested in spliting
the 170k patch for you
[...]
> > Several people including me and vitor try hard to cleanup codecs that had
> > been added without passing through proper review, we really do not need
> > more such code.
>
> What code?
the 170k you commited, i have no interrest in cleaning it up.
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080829/bca22bb6/attachment.pgp>
More information about the ffmpeg-devel
mailing list